1408537 - reftest "font-inflation-1.html" fails on desktop (due to using too small of an epsilon)

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Daniel Holbert [:dholbert]]

The testcase font-inflation-1.html fails for me on Linux desktop (with unmodified current mozilla-central), but it's pretty easy to fix.

The test works by comparing sizes of things (with JS) and verifying that they differ by less than some epsilon.  Right now, epsilon is 2px -- but the "+" icons differ by more than 2px from the reference.  For me, the tested "+" icons are 46.56666564941406 tall, whereas the reference ones are 44px tall. (So they differ by ~2.56666 px)

Let's just increase the epsilon from 2 to 2.6, which is sufficient to get it passing for me. This change shouldn't impact the sensitivity of the test, since it's testing whether or not font inflation applied, and (for me at least) that failure-mode involves a much larger size difference. (E.g. the patch in bug 1373767, which prevents font inflation from applying to one of the elements here, changes its size from ~44px down to ~17px, which is way larger than this proposed 2.6px epsilon.)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: Bug 1408537: Slightly increase the epsilon in MathML reftest "font-inflation-1.html", so that it passes on local linux reftest runs.

The prior epsilon (2px) seems like an arbitrarily chosen value, which must have
been large enough to allow for small sizing differences on our continuous
integration platforms.  But it's not quite large enough to handle sizing
differences on my local linux machine (I'm guessing due to font differences).

Note that this doesn't reduce the sensitivity of the test.  In the test's
anticipated failure mode (when font inflation fails to apply on some element),
the sizing differences are an order of magnitude larger than this amended
epsilon.

Review commit: https://reviewboard.mozilla.org/r/189286/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/189286/

Comment 2

[Daniel Holbert [:dholbert]]

While I'm here, I might as well clean up the misbalanced quote characters, too.

Here's what the test looks like now (it's a bit bigger if you apply font inflation but disregard that for the moment)
https://hg.mozilla.org/mozilla-central/raw-file/196dadb2fe500e75c6fbddcac78106648676cf10/layout/reftests/mathml/font-inflation-1.html

As you can see:
 - The first a+ is quoted
 - The second a+ has an open-quote but no close-quote
 - The third a+ has a close quote (on the following line)
 - The fourth a+ has no quotes.

I'll post a followup patch to consistently quote all of them *except* for the block-level one (because its quotes would end up on separate lines which makes them unnecessary/broken-looking).

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: Bug 1408537 followup: fix unbalanced quotes in prose of reftest font-inflation-1.html.

Review commit: https://reviewboard.mozilla.org/r/189294/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/189294/

Comment 4

[Daniel Holbert [:dholbert]]

Here's a screenshot of what the quotes look like right now:
https://screenshots.firefoxusercontent.com/images/b93f4cf2-c8d4-4f34-88e6-1904c920690b.png
...vs. with my followup fix:
https://screenshots.firefoxusercontent.com/images/d36919c8-5218-4a3a-bef5-0b533d39bf19.png

Comment 5

[Jonathan Kew [:jfkthame]]

Comment on attachment 8918449 [details]
Bug 1408537: Slightly increase the epsilon in MathML reftest "font-inflation-1.html", so that it passes on local linux reftest runs.

https://reviewboard.mozilla.org/r/189286/#review194836

Comment 6

[Jonathan Kew [:jfkthame]]

Comment on attachment 8918453 [details]
Bug 1408537 followup: fix unbalanced quotes in prose of reftest font-inflation-1.html.

https://reviewboard.mozilla.org/r/189294/#review194838

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c6dc1727f18
Slightly increase the epsilon in MathML reftest "font-inflation-1.html", so that it passes on local linux reftest runs. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/eeebce3c103d
followup: fix unbalanced quotes in prose of reftest font-inflation-1.html. r=jfkthame

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/4c6dc1727f18
https://hg.mozilla.org/mozilla-central/rev/eeebce3c103d