Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1569813002:
Make createEvent do more case-insensitive matches (Closed)
|
Created:
4 years, 11 months ago by rwlbuis Modified:
4 years, 11 months ago CC:
chromium-reviews, blink-reviews, tfarina, blink-reviews-w3ctests_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMake createEvent do more case-insensitive matches
We do not support the complete list as shown here:
http://dom.spec.whatwg.org/#dom-document-createevent
Because of this we do not pass the imported test
dom/nodes/Document-createEvent.html.
In order to fix this update make_event_factory.py.
BUG=576764
Committed: https://crrev.com/ae3179bf470da93de141a80b8014b36349a0cf3f
Cr-Commit-Position: refs/heads/master@{#368906}
Patch Set 1 #Patch Set 2 : Remove KeyEvents change #
Messages
Total messages: 27 (15 generated)
Description was changed from ========== createEvent WIP BUG= ========== to ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass dom/nodes/Document-createEvent.html. In order to fix this update make_evenr_factory and make KeyEvents an alias for KeyboardEvent. ==========
Description was changed from ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass dom/nodes/Document-createEvent.html. In order to fix this update make_evenr_factory and make KeyEvents an alias for KeyboardEvent. ========== to ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory and make KeyEvents an alias for KeyboardEvent. ==========
Description was changed from ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory and make KeyEvents an alias for KeyboardEvent. ========== to ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory and make KeyEvents an alias for KeyboardEvent. ==========
Description was changed from ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory and make KeyEvents an alias for KeyboardEvent. ========== to ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory and make KeyEvents an alias for KeyboardEvent. ==========
Description was changed from ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory and make KeyEvents an alias for KeyboardEvent. ========== to ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory and make KeyEvents an alias for KeyboardEvent. ==========
Description was changed from ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory and make KeyEvents an alias for KeyboardEvent. ========== to ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass he imported test dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory and make KeyEvents an alias for KeyboardEvent. ==========
rob.buis@samsung.com changed reviewers: + adamk@chromium.org, philipj@opera.com
rob.buis@samsung.com changed reviewers: + adamk@chromium.org, philipj@opera.com
PTAL.
PTAL.
What is the behavior of other browsers here? I ran http://w3c-test.org/dom/nodes/Document-createEvent.html in Edge and see that the 'KeyEvents' case is failing. If that's only supported by Firefox, I think the spec should be changed instead.
philipj@opera.com changed reviewers: + dtapuska@chromium.org, rbyers@chromium.org
On 2016/01/11 09:18:56, philipj wrote: > What is the behavior of other browsers here? I ran > http://w3c-test.org/dom/nodes/Document-createEvent.html in Edge and see that the > 'KeyEvents' case is failing. If that's only supported by Firefox, I think the > spec should be changed instead. I'll try to find out what Safari does in a bit, from looking at the source it does not seem to support KeyEvents (and maybe not even the case-insensitive behavior). I can of course easily leave that part out of the patch. Not sure what other browsers (or rather engines :)) I can check.
Description was changed from ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass he imported test dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory and make KeyEvents an alias for KeyboardEvent. ========== to ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass the imported test dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory and make KeyEvents an alias for KeyboardEvent. ==========
On 2016/01/11 16:18:48, rwlbuis wrote: > On 2016/01/11 09:18:56, philipj wrote: > > What is the behavior of other browsers here? I ran > > http://w3c-test.org/dom/nodes/Document-createEvent.html in Edge and see that > the > > 'KeyEvents' case is failing. If that's only supported by Firefox, I think the > > spec should be changed instead. > > I'll try to find out what Safari does in a bit, from looking at the source it > does not seem to support KeyEvents (and maybe not even the case-insensitive > behavior). I can of course easily leave that part out of the patch. Not sure > what other browsers (or rather engines :)) I can check. I don't see it supported in Safari, Edge, Internet Explorer or Chrome. I'd prefer to remove it from the DOM spec instead of adding it as it appears to be a Gecko only thing.
On 2016/01/11 17:56:13, dtapuska wrote: > On 2016/01/11 16:18:48, rwlbuis wrote: > > I'll try to find out what Safari does in a bit, from looking at the source it > > does not seem to support KeyEvents (and maybe not even the case-insensitive > > behavior). I can of course easily leave that part out of the patch. Not sure > > what other browsers (or rather engines :)) I can check. > > I don't see it supported in Safari, Edge, Internet Explorer or Chrome. We noticed the same thing with Safari. > I'd prefer to remove it from the DOM spec instead of adding it as it appears to > be a > Gecko only thing. Agreed, I'll remove that part from the patch. Just a pity we can't just delete the test expectation, but the best solution does seem to fix the spec and the test.
Description was changed from ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass the imported test dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory and make KeyEvents an alias for KeyboardEvent. ========== to ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass the imported test dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory. ==========
On 2016/01/11 18:17:25, rwlbuis wrote: > On 2016/01/11 17:56:13, dtapuska wrote: > > On 2016/01/11 16:18:48, rwlbuis wrote: > > > I'll try to find out what Safari does in a bit, from looking at the source > it > > > does not seem to support KeyEvents (and maybe not even the case-insensitive > > > behavior). I can of course easily leave that part out of the patch. Not sure > > > what other browsers (or rather engines :)) I can check. > > > > I don't see it supported in Safari, Edge, Internet Explorer or Chrome. > > We noticed the same thing with Safari. > > > I'd prefer to remove it from the DOM spec instead of adding it as it appears > to > > be a > > Gecko only thing. > > Agreed, I'll remove that part from the patch. Just a pity we can't just delete > the test expectation, but the best solution does seem to fix the spec and the > test. I've filed a spec bug for that: https://github.com/whatwg/dom/issues/148
lgtm, the changes that remain are bringing is closer to Edge. Can you please find or file a bug for this, to keep track of possible reverts? There's of course lots more wrong with createEvent, like the fact that it supports *every* even interface, even new ones, where per spec it should just be the ones in that table.
On 2016/01/12 15:28:17, philipj_OOO_til_Jan_18 wrote: > lgtm, the changes that remain are bringing is closer to Edge. Can you please > find or file a bug for this, to keep track of possible reverts? Thanks! Sure, will make a bug for it. > There's of course lots more wrong with createEvent, like the fact that it > supports *every* even interface, even new ones, where per spec it should just be > the ones in that table. Agreed, that is crbug.com/569690. I can work on it, but the use counter stuff potentially makes this a slow/not so fun issue to work on...
Description was changed from ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass the imported test dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory. ========== to ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass the imported test dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory.py. BUG=576764 ==========
The CQ bit was checked by rob.buis@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1569813002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1569813002/20001
Message was sent while issue was closed.
Description was changed from ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass the imported test dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory.py. BUG=576764 ========== to ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass the imported test dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory.py. BUG=576764 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass the imported test dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory.py. BUG=576764 ========== to ========== Make createEvent do more case-insensitive matches We do not support the complete list as shown here: http://dom.spec.whatwg.org/#dom-document-createevent Because of this we do not pass the imported test dom/nodes/Document-createEvent.html. In order to fix this update make_event_factory.py. BUG=576764 Committed: https://crrev.com/ae3179bf470da93de141a80b8014b36349a0cf3f Cr-Commit-Position: refs/heads/master@{#368906} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/ae3179bf470da93de141a80b8014b36349a0cf3f Cr-Commit-Position: refs/heads/master@{#368906} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
