Scrolling tree should reposition non-stacking order descendents of overflow:scroll
Created attachment 364356 [details] Mostly ready patch, needs tests
rdar://problem/11642295
Attachment 364356 [details] did not pass style-queue: ERROR: Source/WebCore/page/scrolling/mac/ScrollingTreeOverflowScrollingNodeMac.h:53: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/page/scrolling/ScrollingTree.cpp:314: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/page/scrolling/ScrollingTree.cpp:315: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/page/scrolling/ScrollingTree.cpp:318: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/page/scrolling/ScrollingTree.cpp:319: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 5 in 41 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 364356 [details] Mostly ready patch, needs tests Attachment 364356 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11469647 New failing tests: scrollingcoordinator/scrolling-tree/lose-scrolling-node-parent.html scrollingcoordinator/scrolling-tree/reparent-with-layer-removal.html scrollingcoordinator/scrolling-tree/remove-scrolling-role.html scrollingcoordinator/scrolling-tree/reparent-across-compositing-layers.html scrollingcoordinator/scrolling-tree/gain-scrolling-node-parent.html scrollingcoordinator/scrolling-tree/overflow-in-fixed.html
Created attachment 364365 [details] Archive of layout-test-results from ews107 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 364356 [details] Mostly ready patch, needs tests Attachment 364356 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11474453 New failing tests: fast/scrolling/ios/change-scrollability-on-content-resize-nested.html fast/scrolling/ios/change-scrollability-on-content-resize.html scrollingcoordinator/scrolling-tree/lose-scrolling-node-parent.html scrollingcoordinator/scrolling-tree/reparent-with-layer-removal.html scrollingcoordinator/scrolling-tree/remove-scrolling-role.html scrollingcoordinator/scrolling-tree/coordinated-frame-lose-scrolling-ancestor.html scrollingcoordinator/scrolling-tree/reparent-across-compositing-layers.html scrollingcoordinator/scrolling-tree/coordinated-frame-in-fixed.html scrollingcoordinator/scrolling-tree/coordinated-frame.html scrollingcoordinator/scrolling-tree/gain-scrolling-node-parent.html scrollingcoordinator/scrolling-tree/overflow-in-fixed.html scrollingcoordinator/scrolling-tree/coordinated-frame-gain-scrolling-ancestor.html
Created attachment 364389 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Created attachment 364412 [details] Patch
Attachment 364412 [details] did not pass style-queue: ERROR: Source/WebCore/page/scrolling/mac/ScrollingTreeOverflowScrollingNodeMac.h:53: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/page/scrolling/ScrollingTree.cpp:321: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/page/scrolling/ScrollingTree.cpp:322: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/page/scrolling/ScrollingTree.cpp:325: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/page/scrolling/ScrollingTree.cpp:326: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 5 in 48 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 364412 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364412&action=review Looks good. > Source/WebCore/page/scrolling/ScrollingTree.cpp:315 > + auto changeRoot = rootOfScrollingImpactedNodes(changedNode); auto* > Source/WebCore/rendering/RenderLayerBacking.cpp:1833 > + if (roles.contains(ScrollCoordinationRole::Positioning) && m_positioningNodeID) { > + LOG(Compositing, "Detaching Positioned node %" PRIu64, m_positioningNodeID); There is some inconsistency (here and elsewhere) whether these are called "positioning nodes" or "positioned nodes". > Source/WebCore/rendering/RenderLayerCompositor.cpp:2891 > + // FIXME: They should be composited alreayd, but may not have nodeIDs yet. Typo: already > Source/WebCore/rendering/RenderLayerCompositor.cpp:2911 > +// We also have to be positioned, right? indent > Source/WebCore/rendering/RenderLayerCompositor.cpp:3942 > +// FIXME: ordering. indent
Comment on attachment 364412 [details] Patch Attachment 364412 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11478860 New failing tests: scrollingcoordinator/scrolling-tree/positioned-nodes.html scrollingcoordinator/scrolling-tree/gain-scrolling-node-parent.html scrollingcoordinator/scrolling-tree/lose-scrolling-node-parent.html
Created attachment 364429 [details] Archive of layout-test-results from ews106 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 364412 [details] Patch Attachment 364412 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11479512 New failing tests: fast/scrolling/ios/change-scrollability-on-content-resize-nested.html scrollingcoordinator/scrolling-tree/positioned-nodes.html fast/scrolling/ios/change-scrollability-on-content-resize.html scrollingcoordinator/scrolling-tree/lose-scrolling-node-parent.html scrollingcoordinator/scrolling-tree/reparent-with-layer-removal.html scrollingcoordinator/scrolling-tree/remove-scrolling-role.html scrollingcoordinator/scrolling-tree/coordinated-frame-lose-scrolling-ancestor.html scrollingcoordinator/scrolling-tree/reparent-across-compositing-layers.html scrollingcoordinator/scrolling-tree/coordinated-frame-in-fixed.html scrollingcoordinator/scrolling-tree/coordinated-frame.html scrollingcoordinator/scrolling-tree/gain-scrolling-node-parent.html scrollingcoordinator/scrolling-tree/overflow-in-fixed.html scrollingcoordinator/scrolling-tree/coordinated-frame-gain-scrolling-ancestor.html
Created attachment 364445 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Created attachment 364502 [details] Patch
Attachment 364502 [details] did not pass style-queue: ERROR: Source/WebCore/page/scrolling/mac/ScrollingTreeOverflowScrollingNodeMac.h:53: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/page/scrolling/ScrollingConstraints.cpp:106: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/page/scrolling/ScrollingTree.cpp:323: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/page/scrolling/ScrollingTree.cpp:324: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/page/scrolling/ScrollingTree.cpp:327: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/page/scrolling/ScrollingTree.cpp:328: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 6 in 59 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 364502 [details] Patch Attachment 364502 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11485471 New failing tests: scrollingcoordinator/scrolling-tree/positioned-nodes.html compositing/fixed-with-main-thread-scrolling.html scrollingcoordinator/mac/multiple-fixed.html
Created attachment 364510 [details] Archive of layout-test-results from ews105 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 364502 [details] Patch Attachment 364502 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11486774 New failing tests: scrollingcoordinator/ios/nested-fixed-layer-positions.html scrollingcoordinator/scrolling-tree/positioned-nodes.html fast/scrolling/ios/change-scrollability-on-content-resize-nested.html scrollingcoordinator/ios/ui-scroll-fixed.html fast/scrolling/ios/change-scrollability-on-content-resize.html
Created attachment 364519 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 364502 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364502&action=review This patch is huge! > Source/WebCore/page/scrolling/ScrollingCoordinatorTypes.h:41 > + Positioned, extra comma after "Positioned" > Source/WebCore/rendering/RenderLayerCompositor.cpp:2879 > + // FIXME: They should be composited alreayd, but may not have nodeIDs yet. typo: already > Source/WebCore/rendering/RenderLayerCompositor.cpp:2893 > +// This should be: is there an overflow scroll in the composting ancestor chain up to the containing block typo: compositing
Created attachment 364543 [details] Step 1: add positioning nodes
Comment on attachment 364543 [details] Step 1: add positioning nodes I decided to break up the patch.
Comment on attachment 364543 [details] Step 1: add positioning nodes View in context: https://bugs.webkit.org/attachment.cgi?id=364543&action=review > Source/WebCore/page/scrolling/ScrollingConstraints.h:38 > + LayoutConstraints() = default? > Source/WebCore/page/scrolling/ScrollingConstraints.h:62 > + void setLayerPositionAtLastLayout(FloatPoint p) { m_layerPositionAtLastLayout = p; } p???
https://trac.webkit.org/changeset/242913/webkit