Bug 226453

Summary: add Internals method to expose synchronous scrolling reasons for individual scrolling tree scrolling nodes
Product: WebKit Reporter: Cameron McCormack (:heycam) <heycam>
Component: ScrollingAssignee: Cameron McCormack (:heycam) <heycam>
Status: RESOLVED WONTFIX    
Severity: Normal CC: changseok, cmarcelo, darin, esprehn+autocc, ews-watchlist, fred.wang, glenn, jamesr, kondapallykalyan, luiz, pdr, simon.fraser, tonikitoo
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 226399    
Attachments:
Description Flags
Patch fred.wang: review+

Cameron McCormack (:heycam)
Reported 2021-05-30 22:17:58 PDT
So we can test the synchronous scrolling reasons are set correctly on scrolling tree nodes for overflow:scroll elements.
Attachments
Patch (11.78 KB, patch)
2021-05-30 22:20 PDT, Cameron McCormack (:heycam) 		fred.wang: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Cameron McCormack (:heycam)
Comment 1 2021-05-30 22:20:45 PDT
Created attachment 430174 [details] Patch
Frédéric Wang (:fredw)
Comment 2 2021-06-01 06:26:15 PDT
Comment on attachment 430174 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=430174&action=review > Source/WebCore/testing/Internals.h:441 > + ExceptionOr<String> synchronousScrollingReasonsForScrollableElement(Element&) const; Is it possible to make this a const Element&?
Simon Fraser (smfr)
Comment 3 2021-06-01 10:20:38 PDT
Doesn't dumping the scrolling state tree suffice?
Cameron McCormack (:heycam)
Comment 4 2021-06-01 13:59:07 PDT
(In reply to Simon Fraser (smfr) from comment #3) > Doesn't dumping the scrolling state tree suffice? As I discovered from looking at other tests, yes it does. But it includes a lot of other irrelevant information for my tests. Seems like it's better to avoid having to update test expectations for changes in other parts of the scrolling state tree, but if you think it's better to just use the existing thing and avoid adding something new, I can do that.
Darin Adler
Comment 5 2021-06-01 14:31:09 PDT
Comment on attachment 430174 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=430174&action=review >> Source/WebCore/testing/Internals.h:441 >> + ExceptionOr<String> synchronousScrollingReasonsForScrollableElement(Element&) const; > > Is it possible to make this a const Element&? Why does that matter? This is designed to be called from JavaScript.
Cameron McCormack (:heycam)
Comment 6 2021-06-01 15:22:19 PDT
(In reply to Simon Fraser (smfr) from comment #3) > Doesn't dumping the scrolling state tree suffice? I've updated my tests in bug 226399 to use the scrolling state tree dump so I'm not going to use this patch.
Note You need to log in before you can comment on or make changes to this bug.