Closed
Bug 1236076
Opened 9 years ago
Closed 9 years ago
3 tests in text-overflow reftests are failing when run in linux64 docker images
Categories
(Core :: Layout: Text and Fonts, defect)
Core
Layout: Text and Fonts
Tracking
()
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: jmaher, Assigned: jmaher)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
MatsPalmgren_bugz
:
review+
|
Details | Diff | Splinter Review |
in our work to get linux tests running in docker and via taskcluster there are many tests which fail. I am looking at some reftests, and there are 3 which fail in the R2 job related to text-overflow:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=899f61b37a6b&filter-searchStr=r2
In previous bugs we have bumped up the font size and this has resolved issues. I have done that for 3 of the bugs here and it seems to have worked both in the docker image and running locally on my desktop.
Assignee | ||
Comment 1•9 years ago
|
||
:mats, please let me know if this is not the right way to solve this. I don't understand the specific tests, just hacking to get things working.
Comment 2•9 years ago
|
||
Comment on attachment 8703180 [details] [diff] [review]
adjust font-size for the body to fix this.
Review of attachment 8703180 [details] [diff] [review]:
-----------------------------------------------------------------
While it's true that making the font bigger can sometimes be used to resolve issues where overlaps of glyph antialiasing lead to "fuzz", I don't think you can push the sizes anywhere near as big as this, in general. The result will be that a lot of the test content falls outside the window that we capture and compare (I think it used to be 800 x 1000px; not sure if that's still correct as there was talk of reducing the test area at some point), and so the test is no longer testing what was intended. That's certainly the case in these examples; we'd end up only "seeing" a part of the testcase at the top-left.
Attachment #8703180 -
Flags: review?(mats) → review-
Assignee | ||
Comment 3•9 years ago
|
||
a small error leads to big changes! Thanks for pointing it out.
here are the pixels needed to make it pass locally and in the new docker image:
48px - two-value-syntax.html
48px - atomic-under-marker.html
96px - block-padding.html
Please let me know if we should move forward with this, or investigate other ways to solve this. I am really not sure about the block-padding.html case.
Flags: needinfo?(jfkthame)
Comment 4•9 years ago
|
||
Setting font-size:96px on the block-padding.html testcase is definitely out. Try it locally, and you'll see that it blows the size up so big that very little of the testcase remains visible in any reasonable-sized browser window. And it looks like the rendering of the atomic-under-marker.html case also gets kinda weird at 48px, though I haven't looked into why.
From looking at the rendering locally, the only one of these I'd trust to be testing the right thing with the increased font size is two-value-syntax. Even in that case, the test element ends up about 960px high, so it's getting uncomfortably close to exceeding the reftest window size and getting clipped. It's possible that just running on a different platform where the default font has wider line-spacing would be enough to result in losing the bottom elements.
So I think we'd better look into alternative solutions here, sorry. :(
Flags: needinfo?(jfkthame)
Assignee | ||
Comment 5•9 years ago
|
||
the only other solution I know is to fuzzy-if(gtkWidget) on the 3 tests- I am open to trying out other fun stuff though!
Comment 6•9 years ago
|
||
The text-overflow test failures looks like minor anti-aliasing differences to me.
I think fuzzy-if(gtkWidget) is the way to go here.
Comment 7•9 years ago
|
||
Agreed; judging by how they look in reftest-analyzer, I'd go with that.
Assignee | ||
Comment 8•9 years ago
|
||
ok, fuzzy-if to the rescue.
Attachment #8703180 -
Attachment is obsolete: true
Attachment #8703784 -
Flags: review?(mats)
Updated•9 years ago
|
Attachment #8703784 -
Flags: review?(mats) → review+
Comment 10•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in
before you can comment on or make changes to this bug.
Description
•