Summary: | [GTK][WPE] Wrong frame scrolled when view is horizontally scrolled with async scrolling enabled | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||||||
Component: | WebKitGTK | Assignee: | Alejandro G. Castro <alex> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | achristensen, alex, bugs-noreply, clord, cmarcelo, ews-watchlist, fred.wang, jamesr, luiz, mrobinson, simon.fraser, tonikitoo, zan, zdobersek | ||||||||||
Priority: | P2 | Keywords: | Gtk | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Carlos Garcia Campos
2021-03-08 02:48:25 PST
Created attachment 423771 [details]
Patch
Comment on attachment 423771 [details]
Patch
Does this make a test pass?
(In reply to Alex Christensen from comment #2) > Comment on attachment 423771 [details] > Patch > > Does this make a test pass? Great question, I tested gtk and unfourtunately we are not using async in the testing, it is a bummer but it is currently the default so makes sense we still test the other scrolling, that is why we do not get failures in these cases. Hopefully when we have all the situation with AC solved we can release this as a default. Anyway, I'm going to check with wpe because it should be used there and maybe there is some test skipped, we need to do a lot of cleaning there, if there is no test I'll add one. Thanks for the comment. Comment on attachment 423771 [details]
Patch
LGTM, we just need a test
Created attachment 424667 [details]
Patch
Created attachment 424671 [details]
Patch
Uploaded new patch with the test because apparently we were not testing the async code path in the WPE test runner. I've activated the async by default, this can cause some unrelated tests that were passing to fail, so we have to handle this patch with care for WPE. Considering this I've requested a new review from Carlos, I've added also Martin to the CC to add more eyes to check new failures. Regarding the question is we were checking this before I've tested locally the whole fast/scrolling directory and other scrolling tests trying to check if this situation was tested before but just not tested and I could not find it so I added a test for this use case. Apparently the test times out in IOS, probably the scrolling event reaches the external scrollable area instead of the children? I can make it a glib test only because maybe the pixels used for the events in the test do not work well in every platform. I can also skip in that case for IOS if it is interesting. I'm not sure what is the preferred solution here, I'm adding Simon to check the best option in this case. Created attachment 425262 [details]
Patch
Carlos pointed out EventSender's mouseMoveTo is not supported in IOS, so I've skipped it, thanks! zan@falconsigh.net does not have reviewer permissions according to https://raw.githubusercontent.com/WebKit/WebKit/main/Tools/Scripts/webkitpy/common/config/contributors.json. Rejecting attachment 425262 [details] from commit queue. Committed r275593: <https://commits.webkit.org/r275593> All reviewed patches have been landed. Closing bug and clearing flags on attachment 425262 [details]. |