Bug 115824 – [BlackBerry] Don't set active or hover styles on touch start

• Component: UI Events
• Status: RESOLVED INVALID

Setting active or hover styles on links and other parts of the tree can require a relayout and a style recalc which on some pages such as links on http://en.wikipedia.org/wiki/Olympic_Games (desktop version) can take over 300 ms. 
We would like to be able to set elements active if they have a touch listener or are form controls.

Anecdotally, those doing a mouseover on those links on safari desktop results in approx 20ms of style recalc and 20ms of layout which seems high compared to other sites, but not being familiar with its stats I can't tell if its reasonable.

Created attachment 201109 [details]
Patch with changelog

[WebKit Commit Bot]

Attachment 201109 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/page/EventHandler.cpp']" exit_code: 1
Source/WebCore/page/EventHandler.cpp:466:  Do not add platform specific code in WebCore outside of platform.  [build/webcore_platform_layering_violation] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 201109 [details]
Patch with changelog

Attachment 201109 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/428067

[Build Bot]

Comment on attachment 201109 [details]
Patch with changelog

Attachment 201109 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/281496

Created attachment 201284 [details]
Patch with changelog

[WebKit Commit Bot]

Attachment 201284 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/page/EventHandler.cpp']" exit_code: 1
Source/WebCore/page/EventHandler.cpp:466:  Do not add platform specific code in WebCore outside of platform.  [build/webcore_platform_layering_violation] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 201873 [details]
Patch with changelog

[Build Bot]

Comment on attachment 201873 [details]
Patch with changelog

Attachment 201873 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/473636

Created attachment 201876 [details]
Patch with changelog

[Benjamin Poulain]

Comment on attachment 201876 [details]
Patch with changelog

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

> Source/WebCore/ChangeLog:15
> +        Reviewed Internally by Mike Fenton and Mike Lattanzio.
> +        Setting active or hover styles on links can require a style recalculation and a relayout which on some pages such as
> +        http://en.wikipedia.org/wiki/Olympic_Games (desktop version) can take over 300 ms and severly impact our scrolling reaction time.
> +        This optimization still lets us use active styles if the element is a form control (so that we can get depressed
> +        button states) or if the element or its parent has a touch listener (this aligns us with ios and android behaviour.)
> +
> +        No new tests because it shouldn't change behaviour for anybody else.

I sounds more like you have a bad architecture. Why would layout influence the scrolling, I thought you had a multiprocess WebKit on blackberry?

> Source/WebCore/page/efl/EventHandlerEfl.cpp:131
> +    notImplemented();

This is not a notImplemented.

(In reply to comment #10)
> (From update of attachment 201876 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=201876&action=review
> 
> > Source/WebCore/ChangeLog:15
> > +        Reviewed Internally by Mike Fenton and Mike Lattanzio.
> > +        Setting active or hover styles on links can require a style recalculation and a relayout which on some pages such as
> > +        http://en.wikipedia.org/wiki/Olympic_Games (desktop version) can take over 300 ms and severly impact our scrolling reaction time.
> > +        This optimization still lets us use active styles if the element is a form control (so that we can get depressed
> > +        button states) or if the element or its parent has a touch listener (this aligns us with ios and android behaviour.)
> > +
> > +        No new tests because it shouldn't change behaviour for anybody else.
> 
> I sounds more like you have a bad architecture. Why would layout influence the scrolling, I thought you had a multiprocess WebKit on blackberry?

Its because for us touch events are sync, and we want to set the element active/hovered in some cases such as form controls or if there's a touch event handler. So if setting something active such as links requires a big relayout and style recalc, then webkit grinds away doing that before it returns with whether the event has been consumed or not.

As far as I can tell ios safari simply doesn't send touch events to pages with no touch event handlers which prevents elements from being set active which in turn prevents this problem, but we want different behaviour.

> 
> > Source/WebCore/page/efl/EventHandlerEfl.cpp:131
> > +    notImplemented();
> 
> This is not a notImplemented.
Sure, I can remove the notImplemented().