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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Dec 8 02:22:38 PST 2017


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

           What    |Removed                     |Added
 Attachment #328775|review?                     |review+, commit-queue-
              Flags|                            |

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

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

Thanks, I have some minor comments below but otherwise LGTM. It looks like wincairo failed, you should probably consider rebasing your patch against the latest upstream revision.

> Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp:181
> +    

nit: please remove the trailing whitespace.

> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:63
> +    

nit: please remove the trailing whitespace.

> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:75
> +    ASSERT(needsLayout());

Can you please add the following comment:

// We apply horizontal stretchy rules from the MathML spec (sections and, which 
// can be roughly summarized as "stretching operators to the maximum widths of all children" and
// minor variations of that algorithm do not affect the result. However, the spec is a bit ambiguous
// for embellished operators (section and different approaches can lead to significant
// stretch size differences. We made the following decisions:
// - The unstretched size is the embellished operator width with the <mo> at the core unstretched.
// - In general, the target size is just the maximum widths of non-stretchy children because the
//   embellishments could make widths significantly larger.
// - In the edge case when all operators of stretchy, we follow the specification and take the
//   maximum of all unstretched sizes.
// - The <mo> at the core is stretched to cover the target size, even if the embellished operator
//   might become much wider.

> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:76
> +    

nit: trailing whitespace.

> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:81
> +    

nit: trailing whitespace

> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:87
> +            fixLayoutAfterStretch(child, stretchyOperator);

It is a bit sad that we have to do this relayout when we actually only measure the unstretched size for the edge case isAllStretchyOperators == true. Are you able to move that fixLayoutAfterStretch call into the isAllStretchyOperators conditional?

> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:94
> +    

nit: trailing whitespace

> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:99
> +    

nit: trailing whitespace

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/20171208/b5648720/attachment-0001.html>

More information about the webkit-unassigned mailing list