| Summary: | [iOS WK2] REGRESSION (r242687): Programmatic scroll of overflow scroll results in bad rendering | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||||
| Component: | Scrolling | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | dvpdiner2, ews-watchlist, fred.wang, koivisto, rniwa, ryanhaddad, simon.fraser, webkit-bot-watchers-bugzilla, webkit-bug-importer, zalan | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=195507 | ||||||||||||
| Bug Depends on: | 196424 | ||||||||||||
| Bug Blocks: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Simon Fraser (smfr)
2019-03-11 17:06:54 PDT
Marked compositing/ios/overflow-scroll-update-overlap.html as failing for now in https://trac.webkit.org/changeset/242758/webkit We need the same logic we have for "requestedScrollPositionUpdate" for overflow that we have for Frames. Created attachment 364342 [details]
Work in progress
Need to do some cleanup work before this to move isHandlingProgrammaticScroll from ScrollingTree to the scrolling node, and make frames/overflow more similar for programmatic scroll. Might the fix for this also improve the situation for bug 192564 where the UIScrollView contentOffset changing on keyboard closing doesn't result in re-rendering of layers? I don't think so; this is a regression from recent work. Created attachment 366779 [details]
Patch
Comment on attachment 366779 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366779&action=review > Source/WebCore/rendering/RenderLayer.cpp:2817 > + return 0; 0 is a special value? (In reply to zalan from comment #9) > Comment on attachment 366779 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=366779&action=review > > > Source/WebCore/rendering/RenderLayer.cpp:2817 > > + return 0; > > 0 is a special value? Yes, a valid ScrollingNodeID is never 0. (In reply to Simon Fraser (smfr) from comment #10) > (In reply to zalan from comment #9) > > Comment on attachment 366779 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=366779&action=review > > > > > Source/WebCore/rendering/RenderLayer.cpp:2817 > > > + return 0; > > > > 0 is a special value? > > Yes, a valid ScrollingNodeID is never 0. optionals don't work? Comment on attachment 366779 [details] Patch Attachment 366779 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11772321 New failing tests: fast/html/marquee-scrollamount.html Created attachment 366786 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 366779 [details] Patch Attachment 366779 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11772408 New failing tests: fast/html/marquee-scrollamount.html Created attachment 366790 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 366779 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366779&action=review > Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp:163 > + scrollingTree().setIsHandlingProgrammaticScroll(scrollType == ScrollType::Programmatic); Using a tree-global bit as a side channel is bug-prone and confusing. We should probably have some sort of context object that we pass around. (In reply to Antti Koivisto from comment #17) > Comment on attachment 366779 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=366779&action=review > > > Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp:163 > > + scrollingTree().setIsHandlingProgrammaticScroll(scrollType == ScrollType::Programmatic); > > Using a tree-global bit as a side channel is bug-prone and confusing. We > should probably have some sort of context object that we pass around. Agreed that might be nicer, and probably not too bad in the scrolling tree. |