WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
NEW
160509
[meta] Removing/rewriting the mfenced implementation
https://bugs.webkit.org/show_bug.cgi?id=160509
Summary
[meta] Removing/rewriting the mfenced implementation
Frédéric Wang (:fredw)
Reported
2016-08-03 08:00:21 PDT
One of the most serious design issue in the MathML specification is the existence of the mfenced element: * It is strictly equivalent to an expanded form using the mrow and mo elements. There are several complicate equivalence rules to handle and WebKit (or any other implementations) does not support them completely. As a consequence this adds duplicate logic and code to our MathML implementation to support something that does not bring anything new for math rendering or accessibility. * The text in mfenced is contained in the attributes rather than in text nodes. This makes mfenced quite different from the rest of the platform and sharing code even more difficult. This is also the cause of many other bugs or the need for special handling (see below). Issues with this element have been reported to the W3C for a long time but it seems to have been a controversial topic in the Math WG mailing list from its beginning. The last thread is
https://lists.w3.org/Archives/Public/www-math/2016Jul/0004.html
but it seems unlikely that the Math WG will address this anytime soon. The mfenced element is not used in the vast majority of pages or authoring tools (Wikipedia, LibreOffice, latexml, itex2MML, etc). I personally believe the best would just be to remove support for that element in order to clean up our code (Mozilla could do the same in Gecko and other web engines do not support it anyway). If it turned out to be used in old pages then backward compatibility can be ensured by this simple polyfill:
https://people.igalia.com/fwang/mfenced-polyfill/mfenced.js
If we decide to keep it then it will definitely have to be rewritten completely. Here is a non-exhaustive list of issues in WebKit's implementation: I) We do not handle the edge case of empty <mfenced> correctly. This is test 1 of
https://people.igalia.com/fwang/mfenced-polyfill/
II) The MathML specification is not clear about whether open and close fences can have multiple characters but we removed support for this edge case in
r202271
. This is test 10 of
https://people.igalia.com/fwang/mfenced-polyfill/
. open and close fences are still stored on the RenderMathMLFenced class as string. Probably this can just become UChar32 and does not need to be stored on the class. III) Similarly, the parsing of the separators attribute can probably be made more efficient and we may not need to store m_separators on the RenderMathMLFenced class. See also
bug 125938
. IV) We do not handle surrogate pairs in attributes correctly. This is test 15 of
https://people.igalia.com/fwang/mfenced-polyfill/
. See also
bug 125938
. V) RenderMathMLFenced is not updated properly when attributes are modified. See
bug 57696
and
https://bug-57696-attachments.webkit.org/attachment.cgi?id=285222
. VI) RenderMathMLFenced is not updated properly when the child list is modified. See
bug 160507
and
https://bug-160507-attachments.webkit.org/attachment.cgi?id=285227
. VII) Because the text of operators is in attributes they are not searchable/selectable by default. See
bug 139436
. VIII) We use of the old RenderPtr object to hold anonymous operators. See
bug 160396
. IX) The anonymous operators need special handling to work the same as "normal" operators causing additional maintenance cost. This has been quite painful to handle during code refactoring. However, the code for anonymous operators is now better isolated (RenderMathMLFencedOperator.cpp and m_isAnonymousOperator=true in AccessibilityMathMLElement.cpp). Some of the issues above could be fixed by applying the same refactoring as for other MathML classes: 1) Do not create anonymous nodes. This was done for other element in
https://trac.webkit.org/wiki/MathML/Early_2016_Refactoring#Phase1
2) Move attribute parsing from RenderMathMLFenced class to DOM classes. This was done for other elements in
https://trac.webkit.org/wiki/MathML/Early_2016_Refactoring#Phase3
One of the issue with 1) is that the accessibility code assumes the presence of these anonymous operators in the render tree and so removing them without modifying the accessible tree will be tricky. A third option is of course to keep the status quo of mfenced will all its bugs and design issues and hope that it will now no longer interfere too much with the rest of the code.
Attachments
Add attachment
proposed patch, testcase, etc.
Frédéric Wang (:fredw)
Comment 1
2017-11-14 23:53:34 PST
Another issue: X) We don't apply mathvariant/displaystyle to mfenced operators (see tests 18 and 19 in
https://people.igalia.com/fwang/mfenced-polyfill/
). Inconsistencies with normal non-anonymous operators is the cause of
bug 166011
.
Frédéric Wang (:fredw)
Comment 2
2018-05-22 05:54:22 PDT
It seems that some RenderTreeBuilderMathML class was added in
bug 181443
for the sake of RenderMathMLFenced. Dropping mfenced support would probably avoid this code.
Frédéric Wang (:fredw)
Comment 3
2024-10-10 00:04:07 PDT
Copying discussion on slack for the record: ahmad Sep 4th at 2:15 PM I tried deleting mfenced locally as well to clean-up bit of mathml but it led to whole mfenced not rendering. LOL! (this WPT -
https://wpt.fyi/results/mathml/presentation-markup/mrow/legacy-mfenced-element-001.html?label=master&label=experimental&aligned=&q=mfenced
fredw Sep 4th at 2:33 PM Thanks.Regarding `mfenced`, this would be making it an "unknown MathML element" (
https://w3c.github.io/mathml-core/#dfn-unknown-mathml-element
) which MathML Core essentially says they behave as an mrow ; unfortunately I'm not sure WebKit implements that latter behavior yet. fredw Sep 5th at 9:09 AM I just checked this morning and we have a MathMLUnknownElement class already:
https://searchfox.org/wubkat/rev/b36cbce69fddb7da33823f316bd8ead5bebee970/Source/WebCore/mathml/mathtags.in#4Probably
making it derive from MathMLRowElement would be enough so that unknown mathml elements can render:
https://searchfox.org/wubkat/rev/b36cbce69fddb7da33823f316bd8ead5bebee970/Source/WebCore/mathml/MathMLUnknownElement.h#34And
we likely want to remove remove this debug assertion (or override the method in MathMLUnknownElement to create a RenderMathMLRow directly):
https://searchfox.org/wubkat/rev/b36cbce69fddb7da33823f316bd8ead5bebee970/Source/WebCore/mathml/MathMLRowElement.cpp#72
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug