[Webkit-unassigned] [Bug 137330] RenderMathMLUnderOver adds spacing to the child operator indefinitely when resizing the window

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Said Abou-Hallawa <sabouhallawa at apple.com>  2014-10-07 19:48:42 PST ---
In reply to comment #6)
> (From update of attachment 239254 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=239254&action=review
> 
> > Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:77
> > +    Vector<RenderMathMLOperator*> renderOperators;
> 
> Would be more efficient to put an inline capacity here so we don't do memory allocation unless we have an unusually large number of operators.
> 
> > Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:97
> > +    for (auto& renderOperator : renderOperators) {
> > +        renderOperator->stretchTo(stretchWidth);
> >      }
> 
> No braces on a 1-line for statement body in WebKit coding style.

I was testing the fix when I realized we still have a similar problem when zooming the view in or out.  The width of the math operator child of the <munderover> object keeps increasing every time the view is zoomed.  Also I noticed that the math operator does not display its glyph centered under the text (please see the right aroow in the 'Before resizing' image).

So I realized that this fix is not the right one.  I debugged the code more and I found out the real problem. It happens because of using a stall value for the stretch size.  Here is the explanation:

-- The function RenderMathMLUnderOver::layout() does the layout for the <munderover> object in three steps:
  a) It runs the layout for the two children and then gets the maximum size of them.
  b) Then it sets the stretch size for the math operator.
  c) And finally it calls the base class to run the layout for the children again.

** In step a), the layout() of the math operator calls RenderMathMLOperator::computePreferredLogicalWidths() which sets the logical_width = max(stretch_size, glyph_size) + leading_sapce + trailing_sapce. But since stretch_size is not set yet, we end up having: logical_width = glyph_size + leading_sapce + trailing_sapce.

** In step b) we set stretch_size = glyph_size + leading_sapce + trailing_sapce

** In step c), the layout() of the math operator calls RenderMathMLOperator::computePreferredLogicalWidths() a second time we end having:
logical_width = glyph_size + 2 * ( leading_sapce + trailing_sapce ) which is wrong but not very obvious.

-- When the window is resized or the view is zoomed in or out.  The rendering object is marked as 'needsLayout' but the stretch size is not changed.  And here is what happens in RenderMathMLUnderOver::layout() this time:

** In step a) the RenderMathMLOperator needs to run layout, so it calls its layout() function.  When RenderMathMLOperator::computePreferredLogicalWidths() gets called the third time, it uses the old stretch size, so get:
logical_width = max(stretch_size, glyph_size) + leading_sapce + trailing_sapce; which is translated to
logical_width = glyph_size + 2 * (leading_sapce + trailing_sapce)

** In step b) we set stretch_size = glyph_size + 2 * (leading_sapce + trailing_sapce )

** In step c) we end up having logical_width = glyph_size + 3 * ( leading_sapce + trailing_sapce ).

So the more we do the zoom in or out, the more we add spacing to the logical with of the math operator.

The fix is to ensure the stretch size of the math operator render object is reset before running its layout.  Running the layout means either the page is just loaded or it's invalidated which means, no layout stall data should be used.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.