Bug 196142 – [GTK][WPE] Disable "thin", "thick", "medium" values of mfrac@linethickness at runtime

• Status: RESOLVED
• Resolution: FIXED
• Component: MathML

[Frédéric Wang (:fredw)]

The MathML refresh CG decided to remove these values from MathML Core: https://github.com/mathml-refresh/mathml/issues/4

In this bug, we will also introduce a runtime flag to disable more deprecated MathML features later.

[Frédéric Wang (:fredw)]

Created attachment 365712 [details]
Patch

[Frédéric Wang (:fredw)]

Created attachment 365723 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 365723 [details]
Patch

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

> Source/WebCore/mathml/MathMLFractionElement.cpp:58
> +    if (!document().settings().deprecatedMathMLEnabled()) {

This check isn't right.
What we want is something like coreMathMLEnabled, which is disabled by default and enabled on GTK+/WPE.

> Source/WebCore/page/Settings.yaml:824
> +deprecatedMathMLEnabled:
> +  initial: false

We don't want to disable this feature in WebKitLegacy. In fact iBooks uses WebKitLegacy.
In fact, we should keep the initial true and only disable non-core MathML in GTK+/WPE ports.

> Source/WebKit/Shared/WebPreferences.yaml:1636
> +  category: experimental

This categorization isn't right. Having non-Core MathML feature is definitely not experimental.
On the contrary, disabling those features is experimental.
This problem goes away one we replace this with CoreMathMLEnabled instead.

[Frédéric Wang (:fredw)]

Created attachment 365824 [details]
Patch

[Frédéric Wang (:fredw)]

Created attachment 365826 [details]
Patch

[Rob Buis]

Comment on attachment 365826 [details]
Patch

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

Patch LGTM.

> Source/WebCore/ChangeLog:9
> +        adds a condition to disable thin", "thick", "medium" values of

thin misses an opening '"'.

> LayoutTests/imported/w3c/ChangeLog:9
> +        adds a condition to disable thin", "thick", "medium" values of

Ditto.

[Frédéric Wang (:fredw)]

Created attachment 366260 [details]
Patch

[Frédéric Wang (:fredw)]

(In reply to Rob Buis from comment #6)
> Comment on attachment 365826 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=365826&action=review
> 
> Patch LGTM.
> 
> > Source/WebCore/ChangeLog:9
> > +        adds a condition to disable thin", "thick", "medium" values of
> 
> thin misses an opening '"'.
> 
> > LayoutTests/imported/w3c/ChangeLog:9
> > +        adds a condition to disable thin", "thick", "medium" values of
> 
> Ditto.

Thanks. I fixed the change logs and also resolved merge conflicts after https://trac.webkit.org/changeset/243643/webkit ; hopefully this is still working.

[Rob Buis]

Comment on attachment 366260 [details]
Patch

This looks good to me, but I would like others to review the new runtime flag, given earlier discussion.

[Frédéric Wang (:fredw)]

(In reply to Rob Buis from comment #9)
> Comment on attachment 366260 [details]
> Patch
> 
> This looks good to me, but I would like others to review the new runtime
> flag, given earlier discussion.

@Ryosuke Are you happy with the latest version of the patch?

[Frédéric Wang (:fredw)]

Created attachment 368768 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 368768 [details]
Patch

Clearing flags on attachment: 368768

Committed r244869: <https://trac.webkit.org/changeset/244869>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

Please CC webkitgtk-bugs for GTK/WPE bugs.

We need to maintain web compat with Safari, so if Safari will retain these features, that means we should as well, however ill-advised....

[Radar WebKit Bug Importer]

<rdar://problem/50424871>