Bug 226327

Summary: position:sticky content is not correctly offset on async scrolled overflow layers with an RTL vertical scrollbar
Product: WebKit Reporter: Chris Lord <clord>
Component: CompositingAssignee: Chris Lord <clord>
Status: RESOLVED FIXED    
Severity: Normal CC: alex, aperez, cgarcia, cmarcelo, darin, ews-watchlist, fred.wang, jamesr, koivisto, luiz, simon.fraser, tonikitoo, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 224596    
Attachments:
Description Flags
Patch
none
Patch none

Chris Lord
Reported 2021-05-27 05:55:46 PDT
This can be seen in fast/scrolling/rtl-scrollbars-sticky-overflow-scroll-2.html on WPE, where async overflow scrolling is enabled during testing. This seems very similar to bug 210820.
Attachments
Patch (3.13 KB, patch)
2021-06-03 02:06 PDT, Chris Lord 		no flags
DetailsFormatted DiffDiff
Patch (5.19 KB, patch)
2021-06-03 02:10 PDT, Chris Lord 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Chris Lord
Comment 1 2021-06-03 02:02:31 PDT
I believe this does actually affect Mac, but it passes the tests either because async overflow scrolling isn't enabled during testing or because overlay scrollbars are used during testing. Patch incoming.
Chris Lord
Comment 2 2021-06-03 02:06:55 PDT
Created attachment 430452 [details] Patch
Chris Lord
Comment 3 2021-06-03 02:10:56 PDT
Created attachment 430453 [details] Patch
Darin Adler
Comment 4 2021-06-03 07:37:33 PDT
Comment on attachment 430453 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=430453&action=review > Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.mm:80 > + constrainingRect.move(overflowScrollingNode.scrollDeltaSinceLastCommit()); Seems like we should have operator+ for these types so we could do this as expression rather than a “set to wrong value and then fix with a function call”.
Chris Lord
Comment 5 2021-06-03 07:43:20 PDT
(In reply to Darin Adler from comment #4) > Comment on attachment 430453 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=430453&action=review > > > Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.mm:80 > > + constrainingRect.move(overflowScrollingNode.scrollDeltaSinceLastCommit()); > > Seems like we should have operator+ for these types so we could do this as > expression rather than a “set to wrong value and then fix with a function > call”. I guess the reason there isn't is the behaviour of adding a FloatSize to a FloatRect is pretty ambiguous - in this case, we want to offset the origin, but I think adding the size to the size of the rect would be an equally reasonable thing to expect. There is an operator+ for FloatPoint and FloatSize, so I guess this could be FloatRect(m_constraints.constrainingRectAtLastLayout().location() + overflowScrollingNode.scrollDeltaSinceLastCommit(), m_constraints.constrainingRectAtLastLayout().size()) if we wanted to keep it as a one-liner.
Darin Adler
Comment 6 2021-06-03 09:18:50 PDT
(In reply to Chris Lord from comment #5) > I think adding the size to the size of the rect would be an equally > reasonable thing to expect. Interesting point that I hadn’t considered. > There is an operator+ for FloatPoint and FloatSize, so I guess this could be > FloatRect(m_constraints.constrainingRectAtLastLayout().location() + > overflowScrollingNode.scrollDeltaSinceLastCommit(), > m_constraints.constrainingRectAtLastLayout().size()) if we wanted to keep it > as a one-liner. Doesn’t seem better.
Simon Fraser (smfr)
Comment 7 2021-06-03 11:08:16 PDT
I tested the patch on macOS, it seems fine.
Radar WebKit Bug Importer
Comment 8 2021-06-03 11:09:16 PDT
<rdar://problem/78826667>
EWS
Comment 9 2021-06-03 16:39:41 PDT
Committed r278429 (238453@main): <https://commits.webkit.org/238453@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 430453 [details].
Note You need to log in before you can comment on or make changes to this bug.