[Webkit-unassigned] [Bug 181159] Reduce code duplication between RenderMathMLRow, RenderMathMLEnclose, and RenderMathMLRoot

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Jan 28 15:12:00 PST 2018


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

--- Comment #3 from Minsheng Liu <lambda at liu.ms> ---
160547 is almost done so I can work on this now.

1. I am pretty sure some of the change I proposed here will change the behavior of CSS, but they are not currently documented in LayoutTests. My question is, before we take the final action on removing margin/padding/border support in MathML (for now or forever), should we document those changes via tests?

2. A complain about getContentBoundingBox. The style of that function seems so C-ish. Is there a reason not to have something like the following?

struct ContentBoundingBox {
  LayoutUnit ascent, descent, width
};

ContentBoundingBox RenderMathMLRow::contentBoundingBox() const;

3. I think we can combine RenderMathMLRow/Math/Enclose/Root altogether. Code to center children can be done using the same sapceAroundContent virtual method. Hence a timeline:

* Start to move SpaceAroundContent to RenderMathMLRow and simplify initialization of that object. Introduce spaceAroundContent as a virtual method in RenderMathMLRow.
* Reduce code duplication between RenderMathMLRow and RenderMathMLMath.
* Work on RenderMathMLEnclose (and remove cache altogether).
* Work on RenderMathMLRoot (and remove cache altogether).

(In reply to Frédéric Wang (:fredw) from comment #2)
> I would need time to review and think about the whole patch, but I think we
> can already take some of these changes in separate preliminary bugs:
> 
> 1) structures: make the getters const function and initialize constant
> values directly in the *.h. We should check if there are more instances than
> what is done in the patch. For example  e.g.
> 
>    struct SpaceAroundContent {
>         LayoutUnit left = 0;
>         LayoutUnit right = 0;
>         LayoutUnit top = 0;
>         LayoutUnit bottom = 0;
>     };
> 
> 3) Centering <math display="block">: I believe we should actually try to
> move it in the RenderMathMLMath class rather than keeping it in
> RenderMathMLRow.
> 
> 3) Fixing width of <math display="block">:
> https://bugs.webkit.org/show_bug.cgi?id=160547 ; I think this is a serious
> bug and can already be fixed after bug 180348, so I'd propose to try it and
> to write tests before starting the bigger refactoring.
> 
> 4) Removing margin/padding/border and update tests accordingly (
> https://bugs.webkit.org/show_bug.cgi?id=181158 ). I personally think that
> ideally, we should support these CSS properties as they make sense for
> MathML boxes however:
>   - The MathML spec is not clear
>   - Gecko ignores them
>   - WebKit already ignores them for most MathML elements.
>   - It's not used by existing MathML content.
>   So I would agree to remove them for now. My only concern would be that
> maybe some people would use negative margins to workaround the lack of
> negative spacing support (bug 120164 and bug 124830).

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


More information about the webkit-unassigned mailing list