[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

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

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()) {
        box->setNeedsLayout();
        box->layout();
    }

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>