Bug 197492

Summary: [GTK][WPE] Remove attributes deprecated from MathML3
Product: WebKit Reporter: Frédéric Wang (:fredw) <fred.wang>
Component: MathMLAssignee: Rob Buis <rbuis>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dbarton, ews-watchlist, rbuis, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://github.com/mathml-refresh/mathml/issues/5
Bug Depends on:    
Bug Blocks: 195797    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Frédéric Wang (:fredw)
Reported 2019-05-02 02:44:02 PDT
Some attributes were deprecated in MathML3 and have been removed from MathML Core: https://github.com/mathml-refresh/mathml/issues/5#issuecomment-475506856 These might have been implemented in WebKit: fontfamily, fontweight, fontstyle, fontsize, color, background, mode
Attachments
Patch (25.57 KB, patch)
2019-09-09 00:18 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (25.63 KB, patch)
2019-09-09 02:25 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (25.85 KB, patch)
2019-09-09 03:59 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (25.80 KB, patch)
2019-09-09 05:10 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Rob Buis
Comment 1 2019-09-09 00:18:46 PDT
Created attachment 378350 [details] Patch
Rob Buis
Comment 2 2019-09-09 00:43:14 PDT
*** Bug 201583 has been marked as a duplicate of this bug. ***
Rob Buis
Comment 3 2019-09-09 02:25:25 PDT
Created attachment 378358 [details] Patch
Rob Buis
Comment 4 2019-09-09 03:59:38 PDT
Created attachment 378362 [details] Patch
Frédéric Wang (:fredw)
Comment 5 2019-09-09 04:07:23 PDT
Comment on attachment 378362 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378362&action=review > Source/WebCore/ChangeLog:11 > + This chaneg also maps the dir attribute to direction for MathML Core. typo: change ; for all MathML elements. > Source/WebCore/mathml/MathMLElement.cpp:146 > + } else { As I said in private, I'm not really a fan of all these nested if else and addPropertyToPresentationAttributeStyle calls. SVGElement does thing better. But anyway, this is a separate issue that can be handled later. > LayoutTests/imported/w3c/web-platform-tests/mathml/relations/css-styling/attribute-mapping-001.html:1 > +<!DOCTYPE html><!-- webkit-test-runner [ experimental:CoreMathMLEnabled=true ] --> It's a pity we have to modified upstream WPT tests, but that's how WebKit does it currently. For the record, I believe Mozilla and Chromium have separate per-test or per-directory configuration files to enable experimental features.
Rob Buis
Comment 6 2019-09-09 05:10:40 PDT
Created attachment 378364 [details] Patch
WebKit Commit Bot
Comment 7 2019-09-09 08:21:21 PDT
Comment on attachment 378364 [details] Patch Clearing flags on attachment: 378364 Committed r249645: <https://trac.webkit.org/changeset/249645>
WebKit Commit Bot
Comment 8 2019-09-09 08:21:23 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 9 2019-09-09 08:22:17 PDT
<rdar://problem/55182670>
Ryosuke Niwa
Comment 10 2019-09-09 13:00:17 PDT
Comment on attachment 378364 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378364&action=review > Source/WebCore/mathml/MathMLElement.cpp:152 > + // FIXME: The following are deprecated attributes that should lose if there is a conflict with a non-deprecated attribute. > + if (name == fontsizeAttr) I really don't think we should be removing the support for these attributes even if core MathML is enabled. We probably should have a runtime flag separately from coreMathMLEnabled to just disable deprecated attributes, which is probably never enabled in our (Apple) port.
Frédéric Wang (:fredw)
Comment 11 2019-09-09 14:02:50 PDT
Comment on attachment 378364 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378364&action=review >> Source/WebCore/mathml/MathMLElement.cpp:152 >> + if (name == fontsizeAttr) > > I really don't think we should be removing the support for these attributes even if core MathML is enabled. > We probably should have a runtime flag separately from coreMathMLEnabled to just disable deprecated attributes, > which is probably never enabled in our (Apple) port. MathMLCore is currently only used for our (Igalia) ports and we really do want to disable these attributes when MathML Core is enabled and when we run MathML WPT tests. In Mozilla, we chose to introduce one runtime flag / use counter per MathML feature. However introducing runtime flag is a pain in WebKit (especially for WK1 and if we want to expose them in non-macOS UIs), there is no concept of use counter to measure usage statistics and nobody else in WebKit seemed to have interest in removing MathML features from their ports. So it seems easier for us to just go with one flag for now... If someone else really wants to go with multiple flags, they should introduce one per feature and make the MathMLCore one force all of them to be enabled.
Note You need to log in before you can comment on or make changes to this bug.