Bug 220512 – [WPE][GTK] Enable smooth scrolling by default

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Michael Catanzaro]

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.

[Michael Catanzaro]

Created attachment 417384 [details]
Patch

[EWS Watchlist]

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

[Michael Catanzaro]

Ping reviewers.

[Carlos Garcia Campos]

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.

[Michael Catanzaro]

OK.

[Chris Lord]

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.

[Chris Lord]

Created attachment 442012 [details]
Patch

[Carlos Garcia Campos]

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.

[Chris Lord]

Created attachment 442014 [details]
Patch

[EWS]

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].

[Arcady Goldmints-Orlov]

Created attachment 443009 [details]
Test result (note the red pixels)

[Arcady Goldmints-Orlov]

This appears to have cause a regression in the test fast/frames/flattening/scrolling-in-object.html

[Arcady Goldmints-Orlov]

Created attachment 443010 [details]
Expected test result (note no red pixels).

[Chris Lord]

(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.

[Arcady Goldmints-Orlov]

Thanks, I'll take a look at the test and see if I can make some kind of simple test fix.