[Webkit-unassigned] [Bug 179682] Incorrect bounds inside <mover>/<munder> when a stretchy operator is present

[bugzilla-daemon at webkit.org]

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

Frédéric Wang (:fredw) <fred.wang at free.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #327600|review?                     |review-
              Flags|                            |

--- Comment #9 from Frédéric Wang (:fredw) <fred.wang at free.fr> ---
Comment on attachment 327600
  --> https://bugs.webkit.org/attachment.cgi?id=327600
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=327600&action=review

Thanks Minsheng. I think we will really have to rewrite the function (see below). For the tests, you can edit TestExpectations files. There are platform-specific expectations in Layout/platform: 

fred at igalia-fwang:~/src-obj/webkit/WebKit/LayoutTests$ find . -type f -name TestExpectations
./platform/ios-device-wk2/TestExpectations
./platform/gtk-wk2/TestExpectations
./platform/mac/TestExpectations
./platform/wk2/TestExpectations
./platform/ios-wk2/TestExpectations
./platform/ios-simulator/TestExpectations
./platform/ios-wk1/TestExpectations
./platform/ios-simulator-wk1/TestExpectations
./platform/ios-simulator-wk2/TestExpectations
./platform/ios-device/TestExpectations
./platform/gtk/TestExpectations
./platform/mac-wk2/TestExpectations
./platform/ios-device-wk1/TestExpectations
./platform/win/TestExpectations
./platform/gtk-wayland/TestExpectations
./platform/wpe/TestExpectations
./platform/ios/TestExpectations
./platform/mac-wk1/TestExpectations
./platform/mac-elcapitan/TestExpectations
./TestExpectations

And there are corresponding test expectations (e.g. platform/ios/mathml/radical-fallback-expected.txt) that you can update too.

> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:54
>  {

Can you add an ASSERT(isValid()) here? Then we are sure that there are exactly 2 or 3 children.

> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:57
> +    Vector<RenderBox*, 2> stretchingBlocks;

Maybe, we should use 3 as the initial size here, since that's the maximum we can get.

> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:58
>  

So I think we should maybe try to understand the algorithm we want, check with the Math WG and explain with comments here. See https://www.w3.org/TR/MathML3/chapter3.html#id.3.2.5.8.2. For vertical operators they say "cover ... non-stretchy direct sub-expressions" but for horizontal operators, they say "cover ... other direct sub-expressions" which seems inconsistent, we should ask them clarification. Later they indicate that "If a stretchy operator is required to stretch, but all other expressions in the containing element (as described above) are also stretchy, all elements that can stretch should grow to the maximum of the normal unstretched sizes of all elements in the containing object, if they can grow that large". The "Skipping the embellished op" comment suggests that it won't work if we skip non-stretchy (as we do for the vertical rule in RenderMathMLRow::computeLineVerticalStretch) and indeed we should really consider the non-strechy parts of an embellished operators for the size computation. Note that the Math WG is "on hold" ( https://lists.w3.org/Archives/Public/www-math/2016Jul/0001.html ) so we should probably not wait for them to reply soon.

In any case, there is an issue here: The target stretch width calculated here might change after a second layout now that RenderMathMLOperator::stretchTo changes the logical width to give the correct stretched size of the operator instead of the unstretched size. I think we will have no other choice than calculating the target size again with unstretched operators. Performance will probably not be good when we have many nested embellished operators, but that's not common in practice and I think Gecko does something similar. 

So for now in this bug, I think we should just rewrite things that way:

1) If none of the child is a horizontal stretchy operators, then just call layoutIfNeeded() on children and exit.
2) If there is at least one horizontal stretchy operator, then
   * reset the stretch size for all of them and force them to relayout.
   * call layoutIfNeeded on other children.
   * calculate the target width as the maximum of all child->logicalWidth() [note that at this point, the operators are unstretched]
   * call stretchTo on horizontal stretchy operators.
   * force them to relayout again with the correct size [probably you can skip that for direct <mo> children, as you did here].

-- 
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/20171127/59c65462/attachment-0001.html>