Bug 220512 - [WPE][GTK] Enable smooth scrolling by default
Summary: [WPE][GTK] Enable smooth scrolling by default
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P2 Normal
Assignee: Chris Lord
URL: https://gitlab.gnome.org/GNOME/epipha...
Keywords:
Depends on:
Blocks:
 
Reported: 2021-01-11 09:00 PST by Michael Catanzaro
Modified: 2021-11-02 07:58 PDT (History)
16 users (show)

See Also:


Attachments
Patch (3.90 KB, patch)
2021-01-11 09:07 PST, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (6.25 KB, patch)
2021-10-21 05:15 PDT, Chris Lord
no flags Details | Formatted Diff | Diff
Patch (6.17 KB, patch)
2021-10-21 06:21 PDT, Chris Lord
no flags Details | Formatted Diff | Diff
Test result (note the red pixels) (2.86 KB, image/png)
2021-11-01 13:49 PDT, Arcady Goldmints-Orlov
no flags Details
Expected test result (note no red pixels). (2.85 KB, image/png)
2021-11-01 13:50 PDT, Arcady Goldmints-Orlov
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2021-01-11 09:00:13 PST
Since r269143, smooth scrolling should now work for everyone. Let's enable it by default, so applications don't have to think about it anymore.
Comment 1 Michael Catanzaro 2021-01-11 09:07:04 PST
Created attachment 417384 [details]
Patch
Comment 2 EWS Watchlist 2021-01-11 09:08:04 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 Michael Catanzaro 2021-01-18 09:03:35 PST
Ping reviewers.
Comment 4 Carlos Garcia Campos 2021-01-19 00:24:33 PST
Comment on attachment 417384 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=417384&action=review

> Source/WebKit/ChangeLog:9
> +        Smooth scrolling is now enabled by default. This is a minor API break, but that should be OK
> +        since smooth scrolling works much better than it did before.

I'm not sure about this, TBH. I think it's a matter of taste, so I would not change a default behavior that is not clearly an improvement for everybody.
Comment 5 Michael Catanzaro 2021-01-19 06:37:02 PST
OK.
Comment 6 Chris Lord 2021-10-21 04:15:21 PDT
After some discussion, this looks more desirable now. Other platforms have smooth scrolling by default and recent bug-fixes make the experience (subjectively) a lot nicer than without.

It also makes sense to enable this by default to exercise the relevant code-paths more widely. Presumably, platform browsers built on top of the WebKit library will tie the setting to a global platform setting.
Comment 7 Chris Lord 2021-10-21 05:15:08 PDT
Created attachment 442012 [details]
Patch
Comment 8 Carlos Garcia Campos 2021-10-21 05:17:45 PDT
Comment on attachment 442012 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=442012&action=review

> Source/WTF/Scripts/Preferences/WebPreferences.yaml:1906
>      WebKitLegacy:
> -      "PLATFORM(MAC)": true
> +      "PLATFORM(MAC) || PLATFORM(GTK) || PLATFORM(WPE)": true
>        default: false

WPE and GTK don't support legacy webkit.
Comment 9 Chris Lord 2021-10-21 06:21:40 PDT
Created attachment 442014 [details]
Patch
Comment 10 EWS 2021-10-21 07:09:15 PDT
Committed r284603 (243332@main): <https://commits.webkit.org/243332@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 442014 [details].
Comment 11 Arcady Goldmints-Orlov 2021-11-01 13:49:42 PDT
Created attachment 443009 [details]
Test result (note the red pixels)
Comment 12 Arcady Goldmints-Orlov 2021-11-01 13:50:08 PDT
This appears to have cause a regression in the test fast/frames/flattening/scrolling-in-object.html
Comment 13 Arcady Goldmints-Orlov 2021-11-01 13:50:41 PDT
Created attachment 443010 [details]
Expected test result (note no red pixels).
Comment 14 Chris Lord 2021-11-02 02:05:29 PDT
(In reply to Arcady Goldmints-Orlov from comment #12)
> This appears to have cause a regression in the test
> fast/frames/flattening/scrolling-in-object.html

I expect this is down to either the test not waiting long enough for scrolling to finish (it waits 100ms and issues an event that scrolls 160 pixels, I'm not sure what the reasonable wait is for that distance, but it seems it'd be better to hook onto scroll events and wait for the expected value rather than waiting an arbitrary amount of time...) or possibly some kind of rounding error(?).

Easiest thing would be to just disable smooth scrolling for this one test, I don't think it represents a real failure as it is.
Comment 15 Arcady Goldmints-Orlov 2021-11-02 07:58:18 PDT
Thanks, I'll take a look at the test and see if I can make some kind of simple test fix.