[webkit-reviews] review denied: [Bug 180029] MathML Lengths should take zoom level into account : [Attachment 331279] patch

[bugzilla-daemon at webkit.org]

Frédéric Wang (:fredw) <fred.wang at free.fr> has denied Minsheng Liu
<lambda at liu.ms>'s request for review:
Bug 180029: MathML Lengths should take zoom level into account
https://bugs.webkit.org/show_bug.cgi?id=180029

Attachment 331279: patch

https://bugs.webkit.org/attachment.cgi?id=331279&action=review




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

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

> LayoutTests/ChangeLog:10
> +	   * mathml/presentation/mspace-units-expected.html: Remove the call to
setPageZoomFactor().

mspace-units does not have setPageZoomFactor() and I believe the test should
actually *not* be modified as it is not affected by this patch (and please
don't do whitespace only change as that makes the history more complicated).

> LayoutTests/mathml/presentation/mspace-units-with-zoom-expected.html:8
> +    }

That's interesting and seems better than using window.internals. It's the only
difference you make with respect to the mspace-units.html test, right?

> Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp:173
> +

Mmh, that seems complicated. Why don't you just do "* style.effectiveZoom()" in
the 6 cases below? That would make things more readable.