[Webkit-unassigned] [Bug 160509] New: [meta] Removing/rewriting the mfenced implementation

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=160509

            Bug ID: 160509
           Summary: [meta] Removing/rewriting the mfenced implementation
    Classification: Unclassified
           Product: WebKit
           Version: WebKit Nightly Build
          Hardware: All
               URL: https://www.w3.org/TR/MathML3/chapter3.html#presm.mfen
                    ced
                OS: All
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: MathML
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: fred.wang at free.fr
                CC: alex at igalia.com, bfulgham at webkit.org,
                    cfleizach at apple.com, darin at apple.com,
                    jdiggs at igalia.com, jfernandez at igalia.com,
                    rego at igalia.com
        Depends on: 57696, 125938, 139436, 160396, 160507

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.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160803/4a20c600/attachment-0001.html>