Bug 76216 – Focus events should use FocusEvent interface (instead of UIEvents) and expose relatedTarget property

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

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

[Terry Anderson]

I will be taking a look at this

[Eric Seidel (no email)]

Great!  Let us know if we can help.

[Terry Anderson]

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.

[Terry Anderson]

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.

[Terry Anderson]

Created attachment 126378 [details]
Patch

[Eric Seidel (no email)]

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?

[Philippe Normand]

Comment on attachment 126378 [details]
Patch

Attachment 126378 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11487448

[Terry Anderson]

Created attachment 126381 [details]
Patch

[Eric Seidel (no email)]

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.

[Early Warning System Bot]

Comment on attachment 126381 [details]
Patch

Attachment 126381 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/11485417

[Gustavo Noronha (kov)]

Comment on attachment 126381 [details]
Patch

Attachment 126381 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11487457

[Gyuyoung Kim]

Comment on attachment 126381 [details]
Patch

Attachment 126381 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/11487470

[WebKit Review Bot]

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

[Terry Anderson]

Created attachment 126847 [details]
Patch

[Terry Anderson]

Created attachment 126853 [details]
Patch

[Terry Anderson]

Created attachment 127003 [details]
Patch

[Eric Seidel (no email)]

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.

[Terry Anderson]

Created attachment 127030 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 127030 [details]
Patch

LGTM.  Please let the EWS bots munch on it before landing.

[WebKit Review Bot]

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.

[Terry Anderson]

Created attachment 127181 [details]
Patch

[WebKit Review Bot]

Comment on attachment 127181 [details]
Patch

Rejecting attachment 127181 [details] from commit-queue.

[e-mail] 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.

[WebKit Review Bot]

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

[WebKit Review Bot]

The commit-queue encountered the following flaky tests while processing attachment 127181 [details]:

perf/mouse-event.html bug 78736 (author: [e-mail])
inspector/debugger/script-formatter-console.html bug 78737 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Review Bot]

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

[Terry Anderson]

Created attachment 127383 [details]
Patch

[Eric Seidel (no email)]

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.

[WebKit Review Bot]

Comment on attachment 127383 [details]
Patch

Clearing flags on attachment: 127383

Committed r107952: <http://trac.webkit.org/changeset/107952>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Kentaro Hara]

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.)

[Terry Anderson]

(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.

[Kentaro Hara]

Created attachment 185197 [details]
Patch

[Early Warning System Bot]

Comment on attachment 185197 [details]
Patch

Attachment 185197 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/16183073

[Kentaro Hara]

Created attachment 185203 [details]
Patch

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Kentaro Hara]

Created attachment 185233 [details]
Patch

[Terry Anderson]

(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.

[WebKit Review Bot]

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

[Build Bot]

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

[Build Bot]

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

[Sam Weinig]

Out of curiosity, why do we want to support this?  Is there a site that requires it?  Is it a good feature?

[Kentaro Hara]

(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?

[Eric Seidel (no email)]

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. :)

[Eric Seidel (no email)]

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.

[Kentaro Hara]

Because it's in the spec is the only reason for me.

[Eric Seidel (no email)]

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?

[Eric Seidel (no email)]

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. :)

[Eric Seidel (no email)]

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.

[Ryosuke Niwa]

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.

[Eric Seidel (no email)]

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. :)

[Ryosuke Niwa]

(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?

[Kentaro Hara]

Comment on attachment 185233 [details]
Patch

Clearing r? for now, as the topic looks controversial and the patch breaks several tests.

[Sam Weinig]

Heh. Glad I asked.

[Ojan Vafai]

(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?

[Ryosuke Niwa]

(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.

[Kentaro Hara]

(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.

[Kentaro Hara]

Created attachment 186302 [details]
Patch

probably fixed all crashes

[WebKit Review Bot]

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

[Early Warning System Bot]

Comment on attachment 186302 [details]
Patch

Attachment 186302 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/16366139

[Kentaro Hara]

Created attachment 186305 [details]
Patch

[Eric Seidel (no email)]

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?

[Kentaro Hara]

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.)

[Eric Seidel (no email)]

I see, so detail is something that can be set from JS but not ever from our impl?

[Kentaro Hara]

(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.

[Build Bot]

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

[Build Bot]

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

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Kentaro Hara]

Created attachment 186509 [details]
WIP: to see bot results

[WebKit Review Bot]

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

[Kentaro Hara]

(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?

[WebKit Review Bot]

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

[Kentaro Hara]

(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...

[Kentaro Hara]

Created attachment 186829 [details]
another try

[Eric Seidel (no email)]

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. :(

[Christian Biesinger]

*** Bug 100785 has been marked as a duplicate of this bug. ***

[Christian Biesinger]

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;
+};

[Kentaro Hara]

Created attachment 186947 [details]
Patch

[Kentaro Hara]

(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.

[Kentaro Hara]

(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.

[Eric Seidel (no email)]

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 ;

[Kentaro Hara]

Created attachment 186951 [details]
patch for landing

[Ojan Vafai]

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.

[Kentaro Hara]

(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:-)

[Kentaro Hara]

Committed r142072: <http://trac.webkit.org/changeset/142072>