Summary: | REGRESSION (Safari 5.0-5.1): Event.eventPhase is not set to 2 (at target) when handling dblclick event in <input> | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||
Component: | Tools / Tests | Assignee: | Hayato Ito <hayato> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, ap, dglazkov, dominicc, hayato, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 76198 | ||||||||
Attachments: |
|
Description
Eric Seidel (no email)
2012-01-12 14:58:53 PST
Created attachment 122316 [details]
test case from IETC
This used to pass in Safari 5.0. Will look into this. This is a very cool bug. I didn't realize that the meaning of AT_TARGET changes in the presence of the shadow DOM, and the AT_TARGET must be fired more than one time -- once for each shadow boundary. Since we are retargeting events, we must also report each "new" target as a separate occurrence. (In reply to comment #5) > Spec bug: https://www.w3.org/Bugs/Public/show_bug.cgi?id=15543 Let me take this bug. (In reply to comment #6) > > (In reply to comment #5) > > Spec bug: https://www.w3.org/Bugs/Public/show_bug.cgi?id=15543 Created attachment 122611 [details]
multiple AT_TARGET
Although there is on-going discussion on spec bug, https://www.w3.org/Bugs/Public/show_bug.cgi?id=15543, I've implemented mutlple AT_TARGET event mechanism based on my proposal, which fixes the regression. Comment on attachment 122611 [details]
multiple AT_TARGET
nice modification to the test. Ideally, I would like to avoid phase-tweaking logic in EventContext -- to keep class responsibilities clean and simple. Can you file a bug to track this and adjusting to final spec behavior?
Comment on attachment 122611 [details] multiple AT_TARGET View in context: https://bugs.webkit.org/attachment.cgi?id=122611&action=review > Source/WebCore/dom/EventContext.cpp:49 > + if (eventPhase == Event::CAPTURING_PHASE && event->bubbles()) > + return; This return is new. Is it covered by any tests? Comment on attachment 122611 [details] multiple AT_TARGET View in context: https://bugs.webkit.org/attachment.cgi?id=122611&action=review > LayoutTests/fast/dom/shadow/shadow-boundary-events-expected.txt:-80 > -PASS dispatchedEvent("blur") is ["shadowF(@divE)(capturing phase)", "shadowF(@shadowF)(capturing phase)", "shadowG(@shadowG)(capturing phase)", "divH(@divH)"] Right here. Thank you for the review. (In reply to comment #10) > (From update of attachment 122611 [details]) > nice modification to the test. Ideally, I would like to avoid phase-tweaking logic in EventContext -- to keep class responsibilities clean and simple. Can you file a bug to track this and adjusting to final spec behavior? I agree. It seems that EventContext should be a plain data holder class. Let me file another bug to track this. I'll land this patch as is since it got r+. Comment on attachment 122611 [details] multiple AT_TARGET Clearing flags on attachment: 122611 Committed r105123: <http://trac.webkit.org/changeset/105123> All reviewed patches have been landed. Closing bug. |