[Webkit-unassigned] [Bug 170272] MathML: Extra left space for arrows inside <mo>

[bugzilla-daemon at webkit.org]

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

--- Comment #16 from Frédéric Wang (:fredw) <fred.wang at free.fr> ---
(In reply to Minsheng Liu from comment #13)
> Created attachment 326734 [details]
> a proposed patch
> 
> Truly stretched operator requires the special centering code I proposed to
> delete. The reason is that the stretchTo function does not update the
> element’s width (but does change that of m_mathOperator). My proposed patch
> changes that, and everything works fine.
> 
> Regarding to pixel test regression, I got six failed, but five of them
> failed without changing any code, so I am not sure if expected images need
> to be updated. The only case *I think* that is relevant is
> mathml/opentype/opentype-stretchy-horizontal. My patch passed the pixel test
> but has different text output (render tree). Nonetheless, I compared patched
> image output with unmatched output with my eyes and I think the patch gives
> the correct behavior.
> 
> I need the following help:
> 1. I am unsure about how to create a pixel test. I create one, and run the
> pixel test. It seems that WebKit automatically included it into the test
> case. Is that all I need to do? How about other platforms where expected
> images have not been created?
> 
> 2. How to fix the five failed pixel tests. I am pretty sure that my code
> does not change the behavior of WebCore (I compared the result with the
> unmatched build). Do those tests’ expected images need to be updated?
> 
> 3. What else do I need to do/be careful about before we submit the patch?

@Minsheng Liu: Thanks a lot for working on this.

You did not reply, but are you developing on Linux (e.g. WebKitGTK), macOS, or something else? Sometimes the pixel tests require some specific fonts to be installed. Sometimes they have not been updated and thus just marked "failing". I guess you can ignore existing failures, though.

opentype-stretchy-horizontal is supposed to test various ways of stretching operators (base size, variants and assembly). The spacing is irrelevant here and the screenshot you gave still looks good to me. You can check the help of run-webkit-tests to know how re-generate the test expectations for that test (text and png).

You can submit your patch to EWS testing. This will build it on various platforms and run tests on macOS & iOS (with some results and test expectations). You can also take a look at https://mdn.mozillademos.org/en-US/docs/Mozilla/MathML_Project/MathML_Torture_Test$samples/MathML_Torture_Test to see if you don't see anything abnormal or if you patch improves things.

I think we need to add at least one new test for this bug (i.e. that fails before your patch and passes after). The easiest way is to create a pixel test (again the help fo run-webkit-tests explains how to generate test expectations). However, I would prefer javascript tests or reftests, if possible as they are more reliable and easier to debug later. Here are some random ideas:

* A javascript tests that measures sizes & position of some boxes with getBoundingClientRect() to check that the operator is correctly sized/placed. See for example LayoutTests/mathml/presentation/stretchy-depth-height.html for an example.

* A reftest with a rectangle that hides the location where you expect the operator to be. See LayoutTests/mathml/presentation/mroot-transform.html for an example.

* A mismatch reftest for <mo lspace="0px" rspace="50px">→</mo> VS <mo lspace="50px" rspace="0px">→</mo>.

Note: In general try to avoid text in your tests as that may give inaccurate sizes. Use mspace/mpadded with pixel lengths instead. If needed, use the "Ahem" font or some special WOFF fonts to guarantee the expected size.

-- 
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/20171113/274c759d/attachment-0001.html>