Monitor usage of unprefixed and prefixed DOM events for CSS Transitions.

• Component: DOM
• Status: RESOLVED FIXED

[Alexis Menard (darktears)]

Monitor usage of unprefixed and prefixed DOM event for CSS Transitions.

[Alexis Menard (darktears)]

Created attachment 182964 [details]
Patch

[Alexis Menard (darktears)]

Created attachment 182965 [details]
Patch

[Adam Barth]

Comment on attachment 182965 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182965&action=review

> Source/WebCore/dom/EventTarget.cpp:174
> +        if (document->domWindow()) {
> +            if (event->type() == eventNames().transitionendEvent)
> +                FeatureObserver::observe(document->domWindow(), FeatureObserver::UnprefixedTransitionEndEvent);
> +            if (event->type() == eventNames().webkitTransitionEndEvent)
> +                FeatureObserver::observe(document->domWindow(), FeatureObserver::PrefixedTransitionEndEvent);
> +        }

This is fine, but you won't be able to correlate the two.  You might want to also observe when both are registered.

[Adam Barth]

Comment on attachment 182965 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182965&action=review

> Source/WebCore/dom/EventTarget.cpp:164
> +static void observeEvent(const Event* event, const EventTarget* target)

Isn't the common case that event isn't for transitions?  If so, we might want to check for whether the event is transitionendEvent or webkitTransitionEndEvent early in this function.

[Alexis Menard (darktears)]

(In reply to comment #4)
> (From update of attachment 182965 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182965&action=review
> 
> > Source/WebCore/dom/EventTarget.cpp:164
> > +static void observeEvent(const Event* event, const EventTarget* target)
> 
> Isn't the common case that event isn't for transitions?  If so, we might want to check for whether the event is transitionendEvent or webkitTransitionEndEvent early in this function.

Well in that function later I'll put the animation DOM events :).

[Alexis Menard (darktears)]

Comment on attachment 182965 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182965&action=review

>>> Source/WebCore/dom/EventTarget.cpp:164
>>> +static void observeEvent(const Event* event, const EventTarget* target)
>> 
>> Isn't the common case that event isn't for transitions?  If so, we might want to check for whether the event is transitionendEvent or webkitTransitionEndEvent early in this function.
> 
> Well in that function later I'll put the animation DOM events :).

Well in fact before observeEvent is called I check (!prefixedTypeName.isEmpty()) which already filter regular events. You'll be in that function only if the event had a prefixed version.

>> Source/WebCore/dom/EventTarget.cpp:174
>> +        }
> 
> This is fine, but you won't be able to correlate the two.  You might want to also observe when both are registered.

I'm not sure to understand here.

[Adam Barth]

> >> Source/WebCore/dom/EventTarget.cpp:174
> >> +        }
> > 
> > This is fine, but you won't be able to correlate the two.  You might want to also observe when both are registered.
> 
> I'm not sure to understand here.

My understanding is that you're interested in knowing how often web pages are listening for both the prefixed and unprefixed versions of the same event.  The way you've written this patch, you'll learn how often they listening for each type of event, but you won't be able to figure out how often they're listening for *both* at the same time.

[Alexis Menard (darktears)]

Created attachment 183021 [details]
Patch

[Alexis Menard (darktears)]

Created attachment 183026 [details]
Patch

[WebKit Review Bot]

Comment on attachment 183026 [details]
Patch

Clearing flags on attachment: 183026

Committed r139922: <http://trac.webkit.org/changeset/139922>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Lucas Forschler]

Mass move bugs into the DOM component.