Bug 242883 – Remove frame flattening code

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[zalan]

it's unused!

[zalan]

Created attachment 460993 [details]
Patch

[zalan]

for EWS. will also remove tests.

[zalan]

Created attachment 461011 [details]
Patch

[zalan]

Created attachment 461017 [details]
Patch

[zalan]

Created attachment 461029 [details]
Patch

[EWS]

Committed 252632@main (37224f042c17): <https://commits.webkit.org/252632@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 461029 [details].

[Radar WebKit Bug Importer]

<rdar://problem/97298966>

[Milan Crha]

I guess there's no hope to revert this change, is there?

Even you do not use it, the frame flattening is used (and is relied on) in the GNOME Evolution. The removal had been noticed only in the 2.37.90 release of the WebKitGTK [1] and I've got to it only two days before the hard code freeze, which is really not much time to change the core of the app without breaking user experience heavily. The GNOME (and Evolution) code is in the UI freeze already, if it changes anything.

[1] https://gitlab.gnome.org/GNOME/evolution/-/issues/2001

[Michael Catanzaro]

Hi Milan, at your request we going to revert this for 2.38. This is actually public API (WebKitSettings:enable-frame-flattening, webkit_settings_get_enable_frame_flattening(), webkit_settings_set_enable_frame_flattening()), and it's not unused. Apps in Debian that are using it:

asteroidmail
evolution
gnome-feeds
remmina
surf

Ideally we support public APIs forever unless there is a very strong reason not to do so; however, in the case of settings that modify web content display like this one, we need to accept that settings will occasionally stop working as WebKit changes. The commit here deleted nearly 5000 lines of code, and I don't think Zalan is going to want to bring that back. So affected apps will need to stop depending on this setting for WebKitGTK 2.40. WebKitGTK 2.40 will reach stable and LTS Linux distributions around March next year, so affected apps will need to be updated before then ***even in older distributions that would not ordinarily take such updates***.

To give application maintainers some chance to notice that these APIs will break, I will:

 * Deprecate them
 * Backport the deprecation to 2.38
 * Attempt to reach out to the app maintainers

[Michael Catanzaro]

Created heads-up issues:

https://github.com/astroidmail/astroid/issues/720
https://gitlab.com/Remmina/Remmina/-/issues/2780
https://gitlab.gnome.org/World/gfeeds/-/issues/124

Also sent a direct mail to the a Surf developer.

Bug to deprecate this setting: bug #244624

[Michael Catanzaro]

Also note this is exposed via public API on Apple platforms too, see WebKitLegacy/mac/WebView/WebPreferences.mm and WebKitLegacy/mac/WebView/WebView.mm.

[Milan Crha]

> ***even in older distributions that would not ordinarily take such updates***

Well, any such change is against common policies used in such older distributions with or without long term support. If you are going to remove public API, you are required to bump soname version. Everything else comes naturally out of it.

The original change did not bump the soname version, or at least I do not see it in the attached patch, but it's possible I overlooked it.

[Michael Catanzaro]

(In reply to Milan Crha from comment #12)
> > ***even in older distributions that would not ordinarily take such updates***
> 
> Well, any such change is against common policies used in such older
> distributions with or without long term support. If you are going to remove
> public API, you are required to bump soname version. Everything else comes
> naturally out of it.

If we do that, then there's massive breakage and all operating systems will give up on providing our security updates. Keeping the API around as a no-op confines the breakage only to the few apps that are affected by it and allows distros to keep updating WebKitGTK. I doubt we'll *ever* bump the soname for WebKitGTK for GTK 3.

[Michael Catanzaro]

*** Bug 256266 has been marked as a duplicate of this bug. ***