Closed
Bug 1407525
Opened 7 years ago
Closed 7 years ago
stylo: layout/reftests/webkit-box/webkit-display-values-1.html is always failure on Android/stylo
Categories
(Core :: CSS Parsing and Computation, defect, P3)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
People
(Reporter: m_kato, Unassigned)
References
Details
Attachments
(2 files)
https://treeherder.mozilla.org/logviewer.html#?job_id=136096905&repo=try&lineNumber=3449
This may be precision issue for this test and this occurs on both Android/arm and Android/x86 (Android/x86 doesn't run reftest on taskcluster).
This test uses "height: 20px" for div element. But when I modify it to "height: 30px", this test is passed.
Reporter | ||
Comment 1•7 years ago
|
||
Reporter | ||
Comment 2•7 years ago
|
||
Updated•7 years ago
|
Priority: -- → P3
Comment 3•7 years ago
|
||
https://codepen.io/astleychen/pen/RLJBGP
It seems -moz-inline-box is somehow broken now and I'm not sure whether it's something we want to keep the support.
Flags: needinfo?(dholbert)
Comment 4•7 years ago
|
||
ignore not-the-right-fix |
(In reply to Makoto Kato [:m_kato] from comment #0)
> This test uses "height: 20px" for div element. But when I modify it to
> "height: 30px", this test is passed.
That sounds like the right fix to me. The current specified height -- 20px -- is arbitrary (with the intent being that it's larger than the text's height -- but it's apparently not larger than the text, on this platform).
I can make the failure happen locally if I reduce "20px" to e.g. "10px", so it's not really a precision thing or anything too mysterious -- we just do indeed need a larger height.
(In reply to Astley Chen [:astley] (UTC+8) from comment #3)
> It seems -moz-inline-box is somehow broken now
Despite similar naming, -moz-inline-box is completely different under the hood from -webkit-inline-box, so it's not related here. (-moz-box / -moz-inline-box are XUL things and don't play well with HTML, in my experience.)
Flags: needinfo?(dholbert)
Comment 5•7 years ago
|
||
(Thank you for posting that -moz-inline-box testcase, though -- in poking around at it, I discovered that stylo is missing an important-for-mobile -moz/-webkit-*-box quirk. I filed bug 1407701 on that.)
Comment 6•7 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #4)
> (In reply to Astley Chen [:astley] (UTC+8) from comment #3)
> > It seems -moz-inline-box is somehow broken now
>
> Despite similar naming, -moz-inline-box is completely different
Sorry, I was off track here -- I was initially focusing on the test's name, and I skimmed its source too quickly, and I missed that it's testing -webkit-box/-moz-box interactions.
So: since this bug only happens when stylo is enabled, then this is in fact *precisely* bug 1407701, which emilio is fixing soon. I expect this will be fixed when that bug lands.
> I can make the failure happen locally if I reduce "20px" to e.g. "10px", so
> it's not really a precision thing or anything too mysterious -- we just do
> indeed need a larger height.
(This only happens in profiles if stylo is enabled, actually. And the reason it happens is that we're mistakenly honoring "display:-moz-box" on the second div [when stylo is enabled], and "display:-moz-box" behaves kinda like an inline element, so it ends up with some spacing from the line-height at the bottom. Or something like that. Anyway -- the issue goes away and there's no stray spacing if you disable stylo in about:config, even with small "height" values in this testcase.)
Depends on: 1407701
Updated•7 years ago
|
status-firefox57:
--- → wontfix
Reporter | ||
Comment 7•7 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #4)
> I can make the failure happen locally if I reduce "20px" to e.g. "10px", so
> it's not really a precision thing or anything too mysterious -- we just do
> indeed need a larger height.
This occurs when this value is 21 or less than, so it might not precision issue...
Reporter | ||
Comment 8•7 years ago
|
||
If default font is Roboto, it doesn't seem to occur on my test environment. Is this depends on font?
Depends on: 1408847
Comment 9•7 years ago
|
||
Right, it's not a precision issue. And it is font-dependent (indirectly -- really, the issue is whether we end up using our -moz-box codepath, which produces an inline-level element, OR our -webkit-box codepath, which produces a block-level element.
Since the element in question is the only thing on its line (i.e. it's between two block-level things), these two scenarios happen to look the same, *unless* the element happens to be shorter than the line.
Anyway -- this is indicative of a real bug, in that we want to be taking the -webkit-box codepath here. bug 1407701 tracks this and should be the correct way to fix this test failure.
Comment 10•7 years ago
|
||
Fixed by bug 1407701.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•