Summary: | [BlackBerry] Don't set active or hover styles on touch start | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | gmak | ||||||||||
Component: | UI Events | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED INVALID | ||||||||||||
Severity: | Normal | CC: | andersca, buildbot, commit-queue, rniwa | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
gmak
2013-05-08 14:18:18 PDT
Created attachment 201109 [details]
Patch with changelog
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.
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 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
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
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
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(). |