[Webkit-unassigned] [Bug 170272] MathML: Extra left space for arrows inside <mo>

[bugzilla-daemon at webkit.org]

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

--- Comment #23 from Frédéric Wang (:fredw) <fred.wang at free.fr> ---
(In reply to Minsheng Liu from comment #19)
> So,
> 1. I think we should change the current behavior and use the more natural
> width, which involve a non-trivial change to the layout algorithm.
> 2. Since you are more familiar with MathML standard, does the width of a
> stretched operator affect its parents? If the standard is unclear in this
> area, should we send something to the committee?

I believe it indeed *does* affect the width of its parent (otherwise you would paint stretchy operators out of the bounds of munderover) but you can ask confirmation on https://lists.w3.org/Archives/Public/www-math/

BTW, the unofficial draft http://mathml-association.org/MathMLinHTML5/ gives more details in general regarding layout rules.

(In reply to Minsheng Liu from comment #20)
> Created attachment 326859 [details]
> fix the algorithm of layout <mover>/<munder> when stretchy operators are
> present

Thanks for continuing the effort on this, it's very helpful.     

So this patch fixes your initial test case, right? What about the centering, do we still need it? I actually wonder whether it was because the actual size of the stretched operator does not necessarily match the target size (so it would still be more beautiful to do a centering... but ignoring lspace/rspace to avoid this bug)?

I forgot to mention that Tools/Script/check-webkit-style will do the style check of EWS, so you can run it locally. Also, you can use "Tools/Scripts/prepare-ChangeLog" to format a patch for review ("Tools/Scripts/webkit-patch upload" will do that). I guess it is explained in details in https://webkit.org/contributing-code/ 

Checking the code, it seems that RenderMathMLUnderOver::computeOperatorsHorizontalStretch is actually only called in RenderMathMLUnderOver::layoutBlock, just before some calls to layoutIfNeeded() on the munderover children... but I believe these layoutIfNeeded calls are useless as we already do the layout in computeOperatorsHorizontalStretch. Also, computeOperatorsHorizontalStretch really does operator stretching and child layout, so the name is not good (I guess this was inherited from old implementation). So I propose you open a preliminary cleanup bug to rename "computeOperatorsHorizontalStretch" to "stretchHorizontalOperatorsAndLayoutChildren" and remove the useless layoutIfNeeded calls, so that things are a bit clearer. That won't require new tests and you would be able to experience the contribution process.

I quickly try your patch on the torture test and your change makes WebKit crash on an ASSERT for me:

ASSERTION FAILED: !needsLayout()
/Users/fred/WebKit/Source/WebCore/rendering/mathml/RenderMathMLScripts.cpp(479) : virtual std::optional<int> WebCore::RenderMathMLScripts::firstLineBaseline() const
1   0x10eeed24d WTFCrash
2   0x11639fbac WebCore::RenderMathMLScripts::firstLineBaseline() const
3   0x11638b0d8 WebCore::RenderMathMLBlock::ascentForChild(WebCore::RenderBox const&)
4   0x11639b909 WebCore::RenderMathMLRow::firstLineBaseline() const
5   0x11638b0d8 WebCore::RenderMathMLBlock::ascentForChild(WebCore::RenderBox const&)
6   0x11639b909 WebCore::RenderMathMLRow::firstLineBaseline() const
7   0x115ff77a4 WebCore::RenderBlock::firstLineBaseline() const
8   0x116019045 WebCore::RenderBlockFlow::firstLineBaseline() const
9   0x11629a8e6 WebCore::RenderTableCell::cellBaselinePosition() const
10  0x11629ab1b WebCore::RenderTableCell::layout()
11  0x1162b8dcd WebCore::RenderTableRow::layout()
12  0x115f8cdfc WebCore::RenderElement::layoutIfNeeded()
13  0x1162bd020 WebCore::RenderTableSection::layout()
14  0x115f8cdfc WebCore::RenderElement::layoutIfNeeded()
15  0x1162913ba WebCore::RenderTable::layout()

Are you building WebKit in debug mode?  (see https://webkit.org/building-webkit/).

-- 
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/20171114/6abfbb8b/attachment-0001.html>