We currently use a naive approach when computing regions for touch-action since we use the bounds of the element regardless of whether content might overlap or paint outside of its bounds.
<rdar://problem/49516022>
Created attachment 366594 [details] wip
Created attachment 366595 [details] wip
Created attachment 366605 [details] patch
Created attachment 366611 [details] patch
Comment on attachment 366611 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=366611&action=review > Source/WebCore/ChangeLog:15 > + This patch doesn't yes use the computed region for anything except test output. Typo: yet. > Source/WebCore/css/StyleResolver.cpp:853 > + if (effectiveTouchActions.containsAll(elementTouchActions)) > + return elementTouchActions; I don't understand the intent of this code. What is an "effective touch-action"?
> I don't understand the intent of this code. What is an "effective > touch-action"? "Effective touch-action" is what Element::computedTouchActions() used to compute, except now resolved by the style system.
Comment on attachment 366611 [details] patch Attachment 366611 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/11753416 New failing tests: fast/loader/stateobjects/popstate-after-load-complete-addeventlistener.html fast/loader/stateobjects/no-popstate-when-back-to-stateless-entry.html fast/loader/stateobjects/popstate-after-load-complete-body-attribute.html fast/loader/stateobjects/popstate-after-load-complete-body-inline-attribute.html fast/loader/stateobjects/popstate-after-load-complete-window-attribute.html
Created attachment 366624 [details] Archive of layout-test-results from ews549 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews549 Port: win-future Platform: CYGWIN_NT-10.0-2.11.1-0.329-5-3-x86_64-64bit
Comment on attachment 366611 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=366611&action=review > Source/WebCore/css/StyleResolver.cpp:849 > + if (effectiveTouchActions.contains(TouchAction::None)) > + return { TouchAction::None }; So "none" on an ancestor clobbers any value on descendants? That's weird. >> Source/WebCore/css/StyleResolver.cpp:853 >> + return elementTouchActions; > > I don't understand the intent of this code. What is an "effective touch-action"? I'm a bit confused by this code too. I think it needs a link to spec text. > Source/WebCore/css/StyleResolver.cpp:1107 > + style.setEffectiveTouchActions(computeEffectiveTouchActions(style.touchActions(), style.effectiveTouchActions())); This might be clearer if the second param used parentStyle.effectiveTouchActions() > Source/WebCore/rendering/PaintInfo.h:46 > +class TouchActionRegion; Needs if ENABLE(POINTER_EVENTS) guard > Source/WebCore/rendering/PaintInfo.h:135 > + TouchActionRegion* touchActionRegion { nullptr }; Needs if ENABLE(POINTER_EVENTS) guard > Source/WebCore/rendering/RenderLayer.cpp:4873 > + paintInfo.touchActionRegion = localPaintingInfo.touchActionRegion; if ENABLE(POINTER_EVENTS) > Source/WebCore/rendering/RenderLayer.h:81 > +class TouchActionRegion; if ENABLE(POINTER_EVENTS) > Source/WebCore/rendering/RenderLayer.h:918 > + TouchActionRegion* touchActionRegion { nullptr }; if ENABLE(POINTER_EVENTS) > Source/WebCore/rendering/TouchActionRegion.cpp:73 > +constexpr const char* toString(TouchAction touchAction) Or you could just implement TextStream& operator<<(TextStream&, TouchAction) > Source/WebCore/rendering/style/RenderStyle.h:709 > + OptionSet<TouchAction> effectiveTouchActions() const { return OptionSet<TouchAction>::fromRaw(m_rareInheritedData->effectiveTouchActions); } I think this justifies a comment saying that "touch-action is not an inherited property, but we fake one with "effectiveTouchActions" so we can blah blah". > Source/WebCore/rendering/style/StyleRareInheritedData.cpp:373 > + && effectiveTouchActions == o.effectiveTouchActions Do we want this to affect operator==? Will that trigger unwanted layouts/repaints?
I would like to see 2 additional tests: 1. A test for dynamic changes of touch-action (including with nesting), to test that we correctly trigger compositing updates and re-compute the regions. 2. A test for touch-action on descendants of a stacking-context composited layer with negative z-order descendants; in that case we have a more complex layer hierarchy with a "foreground" layer, and we need to ensure that we get the event regions on the right layers.
(In reply to Simon Fraser (smfr) from comment #10) > Comment on attachment 366611 [details] > > > Source/WebCore/css/StyleResolver.cpp:849 > > + if (effectiveTouchActions.contains(TouchAction::None)) > > + return { TouchAction::None }; > > So "none" on an ancestor clobbers any value on descendants? That's weird. That is correct though.
> Do we want this to affect operator==? Will that trigger unwanted > layouts/repaints? No, it is needed for correct operation of these data structures.
Created attachment 366719 [details] patch
> 1. A test for dynamic changes of touch-action (including with nesting), to > test that we correctly trigger compositing updates and re-compute the > regions. Filed https://bugs.webkit.org/show_bug.cgi?id=196608 for this
Comment on attachment 366719 [details] patch Rejecting attachment 366719 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 366719, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Logging in as commit-queue@webkit.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=366719&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=196536&ctype=xml&excludefield=attachmentdata Processing 1 patch from 1 bug. Updating working directory Processing patch 366719 from bug 196536. Fetching: https://bugs.webkit.org/attachment.cgi?id=366719 Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Committing to http://svn.webkit.org/repository/webkit/trunk ... A LayoutTests/pointerevents/ios/touch-action-region-basic-expected.txt A LayoutTests/pointerevents/ios/touch-action-region-basic.html A LayoutTests/pointerevents/ios/touch-action-region-layers-expected.txt A LayoutTests/pointerevents/ios/touch-action-region-layers.html A LayoutTests/pointerevents/ios/touch-action-region-pan-x-y-expected.txt A LayoutTests/pointerevents/ios/touch-action-region-pan-x-y.html A Source/WebCore/rendering/TouchActionRegion.cpp A Source/WebCore/rendering/TouchActionRegion.h M LayoutTests/ChangeLog ERROR from SVN: Item is out of date: File '/trunk/LayoutTests/ChangeLog' is out of date W: bef3df8af37866e14ded5da7f380b344e56e2939 and refs/remotes/origin/master differ, using rebase: :040000 040000 5e7e1b25249eee802961774c39d96156590e3d19 c05e205d0152764832577a6ca506268af3682aa2 M LayoutTests :040000 040000 edf1efeb0534f1659771780e0f6e189c2e629f06 b9cf54b0bba23459a89091da8dfe62e5bdb8c48c M Source Current branch master is up to date. ERROR: Not all changes have been committed into SVN, however the committed ones (if any) seem to be successfully integrated into the working tree. Please see the above messages for details. Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Committing to http://svn.webkit.org/repository/webkit/trunk ... A LayoutTests/pointerevents/ios/touch-action-region-basic-expected.txt A LayoutTests/pointerevents/ios/touch-action-region-basic.html A LayoutTests/pointerevents/ios/touch-action-region-layers-expected.txt A LayoutTests/pointerevents/ios/touch-action-region-layers.html A LayoutTests/pointerevents/ios/touch-action-region-pan-x-y-expected.txt A LayoutTests/pointerevents/ios/touch-action-region-pan-x-y.html A Source/WebCore/rendering/TouchActionRegion.cpp A Source/WebCore/rendering/TouchActionRegion.h M LayoutTests/ChangeLog ERROR from SVN: Item is out of date: File '/trunk/LayoutTests/ChangeLog' is out of date W: bef3df8af37866e14ded5da7f380b344e56e2939 and refs/remotes/origin/master differ, using rebase: :040000 040000 5e7e1b25249eee802961774c39d96156590e3d19 c05e205d0152764832577a6ca506268af3682aa2 M LayoutTests :040000 040000 edf1efeb0534f1659771780e0f6e189c2e629f06 b9cf54b0bba23459a89091da8dfe62e5bdb8c48c M Source Current branch master is up to date. ERROR: Not all changes have been committed into SVN, however the committed ones (if any) seem to be successfully integrated into the working tree. Please see the above messages for details. Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Updating OpenSource From https://git.webkit.org/git/WebKit aedf7fdcc88..580b123afc0 master -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 243879 = aedf7fdcc8890304ec4e2f3fbd2b2444c6c6deb6 r243880 = 580b123afc0ce83c990bee461f3eb6af2d639d68 Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Full output: https://webkit-queues.webkit.org/results/11765928
(In reply to WebKit Commit Bot from comment #16) > ERROR from SVN: > Item is out of date: File '/trunk/LayoutTests/ChangeLog' is out of date > W: bef3df8af37866e14ded5da7f380b344e56e2939 and refs/remotes/origin/master I think this happened because https://trac.webkit.org/changeset/243881/webkit landed and modified LayoutTests/ChangeLog while this CQ action was in flight. It should work if retried.
Comment on attachment 366719 [details] patch Clearing flags on attachment: 366719 Committed r243893: <https://trac.webkit.org/changeset/243893>
All reviewed patches have been landed. Closing bug.
Comment on attachment 366719 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=366719&action=review > Source/WebCore/dom/Element.cpp:-4229 > - for (auto* element = this; element; element = parentCrossingFrameBoundaries(element)) { Removing the usage of parentCrossingFrameBoundaries() broke the iOSMac build. See https://trac.webkit.org/changeset/244046/webkit.