Bug 242883 - Remove frame flattening code
Summary: Remove frame flattening code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
: 256266 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-07-18 19:42 PDT by zalan
Modified: 2023-05-03 13:32 PDT (History)
21 users (show)

See Also:


Attachments
Patch (32.81 KB, patch)
2022-07-18 19:45 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (690.04 KB, patch)
2022-07-19 07:36 PDT, zalan
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (708.01 KB, patch)
2022-07-19 12:00 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (725.39 KB, patch)
2022-07-19 19:59 PDT, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2022-07-18 19:42:09 PDT
it's unused!
Comment 1 zalan 2022-07-18 19:45:47 PDT
Created attachment 460993 [details]
Patch
Comment 2 zalan 2022-07-18 19:57:49 PDT
for EWS. will also remove tests.
Comment 3 zalan 2022-07-19 07:36:23 PDT
Created attachment 461011 [details]
Patch
Comment 4 zalan 2022-07-19 12:00:57 PDT
Created attachment 461017 [details]
Patch
Comment 5 zalan 2022-07-19 19:59:02 PDT
Created attachment 461029 [details]
Patch
Comment 6 EWS 2022-07-19 21:22:18 PDT
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].
Comment 7 Radar WebKit Bug Importer 2022-07-19 21:23:18 PDT
<rdar://problem/97298966>
Comment 8 Milan Crha 2022-08-31 09:09:11 PDT
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
Comment 9 Michael Catanzaro 2022-08-31 12:08:28 PDT
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
Comment 10 Michael Catanzaro 2022-08-31 12:23:19 PDT
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
Comment 11 Michael Catanzaro 2022-08-31 13:53:30 PDT
Also note this is exposed via public API on Apple platforms too, see WebKitLegacy/mac/WebView/WebPreferences.mm and WebKitLegacy/mac/WebView/WebView.mm.
Comment 12 Milan Crha 2022-08-31 22:20:45 PDT
> ***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.
Comment 13 Michael Catanzaro 2022-09-01 05:32:52 PDT
(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.
Comment 14 Michael Catanzaro 2023-05-03 13:32:36 PDT
*** Bug 256266 has been marked as a duplicate of this bug. ***