Summary: | WebKit should unset event propagation flags after dispatch | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||
Component: | DOM | Assignee: | Chris Dumez <cdumez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, commit-queue, darin, dbates, esprehn+autocc, kangil.han, rniwa, sam | ||||||
Priority: | P2 | Keywords: | WebExposed | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
URL: | https://github.com/whatwg/dom/commit/806d4aab584f6fc38c21f8e088b51b8ba3e27e20 | ||||||||
Bug Depends on: | 160980 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Chris Dumez
2016-08-15 09:43:28 PDT
Created attachment 286132 [details]
WIP patch
Looks like some web-platform-tests need to be fixed.
(In reply to comment #1) > Created attachment 286132 [details] > WIP patch > > Looks like some web-platform-tests need to be fixed. https://github.com/w3c/web-platform-tests/pull/3469 Created attachment 286431 [details]
Patch
Comment on attachment 286431 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=286431&action=review > Source/WebCore/dom/EventDispatcher.cpp:190 > - event.setEventPhase(0); > + event.resetPropagationFlags(); > + event.setEventPhase(Event::NONE); Should we combine these two functions? e.g. resetEventPhaseAndPropagationFlags? Comment on attachment 286431 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=286431&action=review >> Source/WebCore/dom/EventDispatcher.cpp:190 >> + event.setEventPhase(Event::NONE); > > Should we combine these two functions? e.g. resetEventPhaseAndPropagationFlags? I don't know if it is worth it. We anyway need to keep the setEventPhase() setter. (In reply to comment #5) > Comment on attachment 286431 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=286431&action=review > > >> Source/WebCore/dom/EventDispatcher.cpp:190 > >> + event.setEventPhase(Event::NONE); > > > > Should we combine these two functions? e.g. resetEventPhaseAndPropagationFlags? > > I don't know if it is worth it. We anyway need to keep the setEventPhase() > setter. But whenever we call resetPropagationFlags, we should be also resetting the phase, right? (In reply to comment #6) > (In reply to comment #5) > > Comment on attachment 286431 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=286431&action=review > > > > >> Source/WebCore/dom/EventDispatcher.cpp:190 > > >> + event.setEventPhase(Event::NONE); > > > > > > Should we combine these two functions? e.g. resetEventPhaseAndPropagationFlags? > > > > I don't know if it is worth it. We anyway need to keep the setEventPhase() > > setter. > > But whenever we call resetPropagationFlags, we should be also resetting the > phase, right? Yes, among other things, like clearing the event path and the currentTarget. Yet I don't feel they are strictly related. From the spec: [...] - Unset event’s dispatch flag, stop propagation flag, and stop immediate propagation flag. - Set event’s eventPhase attribute to NONE. - Set event’s currentTarget attribute to null. - Set event’s path to the empty list. FWIW, I don’t feel strongly about so you can go ahead & land it. It was just a suggestion. Comment on attachment 286431 [details] Patch Rejecting attachment 286431 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 286431, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: org/repository/webkit/trunk ... RA layer request failed: Unable to connect to a repository at URL 'http://svn.webkit.org/repository/webkit/trunk': OPTIONS of 'http://svn.webkit.org/repository/webkit/trunk': could not connect to server (http://svn.webkit.org) at /Applications/Xcode.app/Contents/Developer/usr/libexec/git-core/git-svn line 1032. Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Updating OpenSource Current branch master is up to date. Full output: http://webkit-queues.webkit.org/results/1896706 Comment on attachment 286431 [details] Patch Clearing flags on attachment: 286431 Committed r204630: <http://trac.webkit.org/changeset/204630> All reviewed patches have been landed. Closing bug. |