[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
Sun Dec 3 13:59:27 PST 2017


--- Comment #42 from Darin Adler <darin at apple.com> ---
Comment on attachment 328250
  --> https://bugs.webkit.org/attachment.cgi?id=328250

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

I’d love to say r=me, great that this includes a test and it seems logical, but I am concerned about the setNeedsLayout/layout thing done here.

> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:73
> +    auto* fixingBox = static_cast<RenderBox*>(stretchyOperator);
> +    while (fixingBox != ancestor) {
> +        fixingBox->setNeedsLayout();
> +        fixingBox->layout();
> +        fixingBox = fixingBox->parentBox();
> +    }

I would write:

    RenderBox* fixingBox = stretchyOperator;

Instead of writing the static_cast.

I also would write this as a for loop:

    for (RenderBox* box = stretchyOperator; box != ancestor; box = box->parentBox()) {

But also, it is really strange to do synchronous calls to layout after setNeedsLayout. We need a layout expert to weigh in on this. It seems wrong. Antti? Zalan?

> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:75
> +    ancestor->setNeedsLayout();
> +    ancestor->layout();

Ditto. It seems extremely unlikely that it is OK to do this. Despite the fact that the old code was doing something like this before.

> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:106
> +            child->setNeedsLayout();
>              child->layout();

That same pattern again.

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/20171203/c096cfec/attachment.html>

More information about the webkit-unassigned mailing list