WebKit does not support DOM 3 Events FocusEvent We dispatch blur/focus events, but they are not themselves FocusEvent objects (they're not even UIEvent objects), thus they don't have things like relatedTarget set on them. http://www.w3.org/TR/DOM-Level-3-Events/#webidl-events-FocusEvent I noticed this when investigating these IETC tests: http://samples.msdn.microsoft.com/ietestcenter/domevents/focusin.relatedTarget.html http://samples.msdn.microsoft.com/ietestcenter/domevents/focusout.relatedTarget.html
I will be taking a look at this
Great! Let us know if we can help.
Running http://samples.msdn.microsoft.com/ietestcenter/domevents/focusin.relatedTarget.html on chromium linux: clicking on the button does not actually dispatch a focusin event, but tabbing to the button does.
Created attachment 126303 [details] Layout test for 76216 Layout test to verify that relatedTarget is set appropriately when focusin and focusout events are dispatched by tabbing.
Created attachment 126378 [details] Patch
Comment on attachment 126378 [details] Patch The change looks OK, but the ChangeLog needs work. I assume your'e just moving the *Mediator classes? Can you explain what all your doing?
Comment on attachment 126378 [details] Patch Attachment 126378 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11487448
Created attachment 126381 [details] Patch
Comment on attachment 126381 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126381&action=review So you made no logic changes besides adding the FocusEvent class? Is that correct? > Source/WebCore/ChangeLog:9 > + Created a new FocusEvent class (extends UIEvent) with a relatedTarget attribute. Moved > + the {Focus,Blur,FocusIn,FocusOut}EventDispatchMediator classes inside FocusEvent. Now when > + focusin or focusout events are dispatched, a FocusEvent is created with the relatedTarget > + attribute set accordingly. I probably would have done these two things in separate changes... > Source/WebCore/dom/EventFactory.in:12 > +FocusEvents interfaceName=FocusEvent FocusEvents? I don't see this used. Sounds like a separate change.
Comment on attachment 126381 [details] Patch Attachment 126381 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11485417
Comment on attachment 126381 [details] Patch Attachment 126381 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11487457
Comment on attachment 126381 [details] Patch Attachment 126381 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11487470
Comment on attachment 126381 [details] Patch Attachment 126381 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11496118 New failing tests: fast/dom/shadow/shadow-boundary-events.html
Created attachment 126847 [details] Patch
Created attachment 126853 [details] Patch
Created attachment 127003 [details] Patch
Comment on attachment 127003 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127003&action=review The diff is a bit broken, but otherwise looks OK. Upload a new patch and we're good to go. > Source/WebCore/WebCore.xcodeproj/project.pbxproj:996 > - 4123081B138C429700BCCFCA /* WebCore.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 93F19B1A08245E5A001E9ABC /* WebCore.framework */; }; > + 4123081B138C429700BCCFCA /* .framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 93F19B1A08245E5A001E9ABC /* .framework */; }; I think this is an error. > Source/WebCore/WebCore.xcodeproj/project.pbxproj:7819 > - 417DA6D013734E02007C57FB /* libWebCoreTestSupport.dylib */ = {isa = PBXFileReference; explicitFileType = "compiled.mach-o.dylib"; includeInIndex = 0; path = libWebCoreTestSupport.dylib; sourceTree = BUILT_PRODUCTS_DIR; }; > + 417DA6D013734E02007C57FB /* .dylib */ = {isa = PBXFileReference; explicitFileType = "compiled.mach-o.dylib"; includeInIndex = 0; path = .dylib; sourceTree = BUILT_PRODUCTS_DIR; }; > 417DA71B13735DFA007C57FB /* JSInternals.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSInternals.cpp; sourceTree = "<group>"; }; This too. > Source/WebCore/WebCore.xcodeproj/project.pbxproj:10142 > - 93F19B1A08245E5A001E9ABC /* WebCore.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = WebCore.framework; sourceTree = BUILT_PRODUCTS_DIR; }; > + 93F19B1A08245E5A001E9ABC /* .framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = .framework; sourceTree = BUILT_PRODUCTS_DIR; }; Same error. > Source/WebCore/WebCore.xcodeproj/project.pbxproj:24278 > +<<<<<<< HEAD > + B6D9D23514EABD260090D75E /* FocusEvent.h in Headers */, > + B6D9D27B14EAC0860090D75E /* JSFocusEvent.h in Headers */, > +======= > C598905714E9C28000E8D18B /* PasteboardStrategy.h in Headers */, > C598905814E9C29900E8D18B /* PlatformPasteboard.h in Headers */, > +>>>>>>> gclient Merge confict.
Created attachment 127030 [details] Patch
Comment on attachment 127030 [details] Patch LGTM. Please let the EWS bots munch on it before landing.
Attachment 127003 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9 Updating OpenSource From git://git.webkit.org/WebKit a9730c4..2ce77d7 master -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 107794 = c59b9dab74417b86c7b5b60a4408ab3e5d1659a8 r107793 = 2ce77d76874b9ee77991fecb540696485733202a Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc M Source/WebCore/ChangeLog M Source/WebCore/platform/graphics/FontCache.cpp 107794 = c59b9dab74417b86c7b5b60a4408ab3e5d1659a8 already exists! Why are we refetching it? at /usr/lib/git-core/git-svn line 5210 Died at Tools/Scripts/update-webkit line 164. If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 127181 [details] Patch
Comment on attachment 127181 [details] Patch Rejecting attachment 127181 [details] from commit-queue. tdanderson@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
Comment on attachment 127181 [details] Patch Rejecting attachment 127181 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: sing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 164. Failed to run "['Tools/Scripts/update-webkit', '--chromium', '--force-update']" exit_code: 9 ntinue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 164. Full output: http://queues.webkit.org/results/11532272
The commit-queue encountered the following flaky tests while processing attachment 127181 [details]: perf/mouse-event.html bug 78736 (author: eae@chromium.org) inspector/debugger/script-formatter-console.html bug 78737 (author: pfeldman@chromium.org) The commit-queue is continuing to process your patch.
Comment on attachment 127181 [details] Patch Rejecting attachment 127181 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: kit-commit-queue/Source/WebKit/chromium/tools/win --revision 122122 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 43>At revision 122122. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://queues.webkit.org/results/11527929
Created attachment 127383 [details] Patch
Comment on attachment 127383 [details] Patch The cq got sick yesterday with the git.webkit.org outage. The failures had nothing to do with your patch as far as I can tell.
Comment on attachment 127383 [details] Patch Clearing flags on attachment: 127383 Committed r107952: <http://trac.webkit.org/changeset/107952>
All reviewed patches have been landed. Closing bug.
This change has been rolled out in r108034. Do you have any plan to support FocusEvent in WebKit? (I'm asking this just because I wanted to implement a constructor for FocusEvent.)
(In reply to comment #30) > This change has been rolled out in r108034. > > Do you have any plan to support FocusEvent in WebKit? > > (I'm asking this just because I wanted to implement a constructor for FocusEvent.) This was my starter bug from a year ago and to be honest I didn't realize it got rolled out until seeing your comment here! If you want to take ownership of this issue and try to re-land this patch, please feel free to assign yourself to the bug.
Created attachment 185197 [details] Patch
Comment on attachment 185197 [details] Patch Attachment 185197 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/16183073
Created attachment 185203 [details] Patch
Comment on attachment 185203 [details] Patch Attachment 185203 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16115145 New failing tests: fast/events/related-target-focusevent.html fast/forms/month-multiple-fields/month-multiple-fields-blur-and-focus-events.html fast/forms/time-multiple-fields/time-multiple-fields-state-change-on-focus-or-blur.html fast/forms/time-multiple-fields/time-multiple-fields-blur-and-focus-events.html fast/forms/date-multiple-fields/date-multiple-fields-blur-and-focus-events.html fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-blur-and-focus-events.html fast/dom/shadow/shadow-boundary-events.html fast/forms/week-multiple-fields/week-multiple-fields-blur-and-focus-events.html
Comment on attachment 185203 [details] Patch Attachment 185203 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16115169 New failing tests: fast/events/related-target-focusevent.html fast/forms/month-multiple-fields/month-multiple-fields-blur-and-focus-events.html fast/forms/time-multiple-fields/time-multiple-fields-state-change-on-focus-or-blur.html fast/forms/time-multiple-fields/time-multiple-fields-blur-and-focus-events.html fast/forms/date-multiple-fields/date-multiple-fields-blur-and-focus-events.html fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-blur-and-focus-events.html fast/dom/shadow/shadow-boundary-events.html fast/forms/week-multiple-fields/week-multiple-fields-blur-and-focus-events.html
Created attachment 185233 [details] Patch
(In reply to comment #37) > Created an attachment (id=185233) [details] > Patch Be sure to keep an eye on the two chromium mac tests that failed for my patch: https://bugs.webkit.org/show_bug.cgi?id=78832. From looking at the logs, EWS didn't catch fast/dom/shadow/shadow-boundary-events.html.
Comment on attachment 185233 [details] Patch Attachment 185233 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16203186 New failing tests: fast/forms/month-multiple-fields/month-multiple-fields-blur-and-focus-events.html fast/forms/time-multiple-fields/time-multiple-fields-state-change-on-focus-or-blur.html fast/forms/time-multiple-fields/time-multiple-fields-blur-and-focus-events.html fast/forms/date-multiple-fields/date-multiple-fields-blur-and-focus-events.html fast/dom/shadow/shadow-boundary-events.html fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-blur-and-focus-events.html fast/forms/week-multiple-fields/week-multiple-fields-blur-and-focus-events.html
Comment on attachment 185233 [details] Patch Attachment 185233 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16203237 New failing tests: fast/events/related-target-focusevent.html
Comment on attachment 185233 [details] Patch Attachment 185233 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16201266 New failing tests: fast/events/related-target-focusevent.html
Out of curiosity, why do we want to support this? Is there a site that requires it? Is it a good feature?
(In reply to comment #42) > Out of curiosity, why do we want to support this? Is there a site that requires it? Is it a good feature? Terry, Eric: do you have any background context?
I filed this based on IE Test Center tests. It's not at all clear to me that we want this. Just because IE has it doesn't mean we should. :)
That said... it does kinda make sense that an Event would have its own class. At least a DOM-level event generated by the browser. :) My position was one of ambivalence. I'm really neither for nor against w/o further research.
Because it's in the spec is the only reason for me.
I just tested in Chrome and FF on my Mac and neither of them even seemed to dispatch the focus event for the button. I'm not sure we send focus events for buttons on Mac?
The focusout test however, results in a UIEvent in Chrome, but nothing in Firefox. It would appear that focus events are not universally supported in the first place, let alone with the spec'd prototype chain. :)
Again, I feel il-informed, and would love comment from one of the spec authors. Or even what they believe their implementation status is. It seems fine to wait on this bug until something actually needs it (which is inevitable now that one major browser supports these). As noted from my testing above, it's possible that focus events don't even make sense on a system like Mac where buttons don't get focused from clicks! Interestingly, testing fucus using tab (which can focus a button) doesn't dispatch the events in FF either, but again, does in Chrome, just with the "wrong" type.
Adding focusin/focusout may introduce new security vulnerabilities if they can fire when focus/blur doesn't fire. In general, these synchronous focus related events are evil and we should not try to support them.
I agree synchronous JS events are bad-news. I should clarify that this bug is about what prototype chain we have on the event we send, whether or not we send it, btw. :)
(In reply to comment #51) > I agree synchronous JS events are bad-news. I should clarify that this bug is about what prototype chain we have on the event we send, whether or not we send it, btw. :) We should probably rename the bug title then?
Comment on attachment 185233 [details] Patch Clearing r? for now, as the topic looks controversial and the patch breaks several tests.
Heh. Glad I asked.
(In reply to comment #42) > Out of curiosity, why do we want to support this? Is there a site that requires it? Is it a good feature? It's a good feature and we absolutely should do it. Knowing which element is getting blurred during a focus event (and vice versa) is very useful and practically impossible to keep track of in JS code. I don't know of a site that requires it, but we get web developers asking for this all the time. (In reply to comment #53) > (From update of attachment 185233 [details]) > Clearing r? for now, as the topic looks controversial and the patch breaks several tests. Lets try to get this in. There's no controversy. The working group discussed this and there was all-around support. Also, this is a dupe of bug 100785. The approach in bug 100785 seems much simpler. Is there a case that approach gets wrong?
(In reply to comment #55) > (In reply to comment #42) > > Out of curiosity, why do we want to support this? Is there a site that requires it? Is it a good feature? > > It's a good feature and we absolutely should do it. Knowing which element is getting blurred during a focus event (and vice versa) is very useful and practically impossible to keep track of in JS code. > > I don't know of a site that requires it, but we get web developers asking for this all the time. > > (In reply to comment #53) > > (From update of attachment 185233 [details] [details]) > > Clearing r? for now, as the topic looks controversial and the patch breaks several tests. > > Lets try to get this in. There's no controversy. The working group discussed this and there was all-around support. Also, this is a dupe of bug 100785. The approach in bug 100785 seems much simpler. Is there a case that approach gets wrong? Yeah, my concern was if we were adding new event or firing it often. It seems like we're not doing that, so using the right interface object seems like an obvious improvement.
(In reply to comment #55) > Lets try to get this in. There's no controversy. The working group discussed this and there was all-around support. Also, this is a dupe of bug 100785. The approach in bug 100785 seems much simpler. Is there a case that approach gets wrong? The approach looks good but it looks like our approaches are the same. Tests about relatedTarget of focus/blur/focusin/focusout events are failing on both of our patches. Let me polish them up.
Created attachment 186302 [details] Patch probably fixed all crashes
Comment on attachment 186302 [details] Patch Attachment 186302 [details] did not pass cr-linux-debug-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16369051
Comment on attachment 186302 [details] Patch Attachment 186302 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/16366139
Created attachment 186305 [details] Patch
Comment on attachment 186305 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186305&action=review > Source/WebCore/WebCore.xcodeproj/project.pbxproj:19541 > + B6D9D27A14EAC0860090D75E /* JSFocusEvent.cpp */, > + B6D9D27914EAC0860090D75E /* JSFocusEvent.h */, Looks like hte indent is wrong. > Source/WebCore/dom/FocusEvent.cpp:42 > +}; Extra ; > Source/WebCore/dom/FocusEvent.h:54 > + FocusEvent(const AtomicString& type, bool canBubble, bool cancelable, PassRefPtr<AbstractView>, int, PassRefPtr<EventTarget>); What is int detail? Is it ever used?
Comment on attachment 186305 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186305&action=review >> Source/WebCore/WebCore.xcodeproj/project.pbxproj:19541 >> + B6D9D27914EAC0860090D75E /* JSFocusEvent.h */, > > Looks like hte indent is wrong. Will fix. >> Source/WebCore/dom/FocusEvent.h:54 >> + FocusEvent(const AtomicString& type, bool canBubble, bool cancelable, PassRefPtr<AbstractView>, int, PassRefPtr<EventTarget>); > > What is int detail? Is it ever used? detail is an attribute of UIEvent (https://dvcs.w3.org/hg/d4e/raw-file/tip/source_respec.htm#constructor-uievent). Regarding focusin/focusout events triggered inside WebCore, detail is always set to 0. (Please look at the diff of Node.cpp.)
I see, so detail is something that can be set from JS but not ever from our impl?
(In reply to comment #64) > I see, so detail is something that can be set from JS but not ever from our impl? Yes, I think so.
Comment on attachment 186305 [details] Patch Attachment 186305 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16372095 New failing tests: fast/events/related-target-focusevent.html
Comment on attachment 186305 [details] Patch Attachment 186305 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16371107 New failing tests: fast/events/related-target-focusevent.html
Comment on attachment 186305 [details] Patch Attachment 186305 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16377015 New failing tests: fast/dom/shadow/shadow-boundary-events.html
Comment on attachment 186305 [details] Patch Attachment 186305 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16368140 New failing tests: fast/dom/shadow/shadow-boundary-events.html
Created attachment 186509 [details] WIP: to see bot results
Comment on attachment 186509 [details] WIP: to see bot results Attachment 186509 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16366568 New failing tests: fast/dom/shadow/shadow-boundary-events.html
(In reply to comment #71) > (From update of attachment 186509 [details]) > Attachment 186509 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/16366568 > > New failing tests: > fast/dom/shadow/shadow-boundary-events.html Now it looks like that this is the only failure. I don't understand why the test fails in the chromium-linux bot even though the test passes in my local linux release/debug environment. Any idea?
Comment on attachment 186509 [details] WIP: to see bot results Attachment 186509 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16375517 New failing tests: fast/dom/shadow/shadow-boundary-events.html
(In reply to comment #73) > (From update of attachment 186509 [details]) > Attachment 186509 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/16375517 > > New failing tests: > fast/dom/shadow/shadow-boundary-events.html Is there any way to retrieve the bot result? Given that the test is passing on my local chromium-release build, there is no hint for debugging...
Created attachment 186829 [details] another try
Comment on attachment 186829 [details] another try Lots of tabs in the XCode file. :( I know XCode is a big pain in the ass to edit, sorry. :(
*** Bug 100785 has been marked as a duplicate of this bug. ***
Do you not need the event constructor attributes in the IDL file? Like what my patch had: +[ + ConstructorConditional=DOM4_EVENTS_CONSTRUCTOR, + ConstructorTemplate=Event +] +interface FocusEvent : UIEvent +{ + [InitializedByEventConstructor] readonly attribute EventTarget relatedTarget; +};
Created attachment 186947 [details] Patch
(In reply to comment #76) > (From update of attachment 186829 [details]) > Lots of tabs in the XCode file. :( I know XCode is a big pain in the ass to edit, sorry. :( eric: Thanks. Fixed the tab issue. Now bots are green.
(In reply to comment #78) > Do you not need the event constructor attributes in the IDL file? Like what my patch had: Let me do it in a follow-up patch with test cases. I'll also upload a couple of follow-up patches for refactoring.
Comment on attachment 186947 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186947&action=review This looks fantastic! > Source/WebCore/dom/FocusEvent.cpp:42 > +}; Extra ;
Created attachment 186951 [details] patch for landing
I think we should support these on focus/blur as well. See http://lists.w3.org/Archives/Public/www-dom/2012OctDec/0061.html for the www-dom discussion. Doesn't need to block this patch, but it'd be a nice followup.
(In reply to comment #84) > I think we should support these on focus/blur as well. See http://lists.w3.org/Archives/Public/www-dom/2012OctDec/0061.html for the www-dom discussion. Will do! (though I hit a bunch of crashes when I tried it:-)
Committed r142072: <http://trac.webkit.org/changeset/142072>