[Webkit-unassigned] [Bug 180029] MathML Lengths should take zoom level into account

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Jan 13 02:31:39 PST 2018


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

--- Comment #5 from Minsheng Liu <lambda at liu.ms> ---
(In reply to Frédéric Wang (:fredw) from comment #4)
> Comment on attachment 331279 [details]
> 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?

Oops! I mistook the file you provided on this page with the one we have in the repository.

Note that the old test does not check the unit "mu". I agree you with that a significant change to whitespace is not history-friendly, something I have not thought of before. So I will submit a separate patch to amend that test later. For this patch, I will only include the new mspace-units-with-zoom.

And yes, other than the CSS property "zoom" and the check for "mu", there is nothing new.

> > 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.

I had a hard time deciding between code duplication and readability. If we are using Rust or other switch-as-expression languages, I will try to argue. However, since I am duplicating lots of "break" anyway, I agree with your proposal.

-- 
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/20180113/695202c9/attachment-0001.html>


More information about the webkit-unassigned mailing list