Closed Bug 1533392 Opened 6 months ago Closed 6 months ago

Intermittent layout/style/test/test_revert.html | font-family: Should inherit something non-initial to begin with - didn't expect "sans-serif", but got it

Categories

(Core :: CSS Parsing and Computation, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox-esr60 --- unaffected
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: emilio)

References

Details

(Keywords: intermittent-failure, regression)

Attachments

(1 file)

#[markdown(off)]
Filed by: aciure [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=232413050&repo=autoland

https://queue.taskcluster.net/v1/task/UGCt3S8_QSOzSym649Ebwg/runs/0/artifacts/public/logs/live_backing.log

[task 2019-03-07T13:35:33.931Z] 13:35:33 INFO - 97 INFO TEST-PASS | layout/style/test/test_revert.html | float: Should get something non-initial to begin with
[task 2019-03-07T13:35:33.932Z] 13:35:33 INFO - 98 INFO TEST-PASS | layout/style/test/test_revert.html | float: Should behave as if there was no author style
[task 2019-03-07T13:35:33.932Z] 13:35:33 INFO - 99 INFO TEST-PASS | layout/style/test/test_revert.html | font: Should inherit something non-initial to begin with
[task 2019-03-07T13:35:33.934Z] 13:35:33 INFO - 100 INFO TEST-PASS | layout/style/test/test_revert.html | font: Should get the non-inherited value from somewhere (expected inheritance - inherited: italic ; historical-forms ; small-caps ; jis78 ; none ; lining-nums ; super ; 700 ; 18px ; 18px ; sans-serif ; 50% ; 0.3 ; "liga" ; "ENG" ; normal ; historical-forms ; small-caps ; jis78 ; none ; lining-nums ; super ; "wdth" 0 ; none - parent: italic ; historical-forms ; small-caps ; jis78 ; none ; lining-nums ; super ; 700 ; 18px ; 18px ; sans-serif ; 50% ; 0.3 ; "liga" ; "ENG" ; normal ; historical-forms ; small-caps ; jis78 ; none ; lining-nums ; super ; "wdth" 0 ; none - default: normal ; normal ; normal ; normal ; normal ; normal ; normal ; 400 ; 16px ; 19px ; sans-serif ; 100% ; none ; normal ; normal ; auto ; normal ; normal ; normal ; normal ; normal ; normal ; normal ; auto)
[task 2019-03-07T13:35:33.934Z] 13:35:33 INFO - 101 INFO TEST-PASS | layout/style/test/test_revert.html | font: Should behave as if there was no author style
[task 2019-03-07T13:35:33.935Z] 13:35:33 INFO - Buffered messages finished
[task 2019-03-07T13:35:33.935Z] 13:35:33 INFO - 102 INFO TEST-UNEXPECTED-FAIL | layout/style/test/test_revert.html | font-family: Should inherit something non-initial to begin with - didn't expect "sans-serif", but got it
[task 2019-03-07T13:35:33.936Z] 13:35:33 INFO - SimpleTest.isnot@SimpleTest/SimpleTest.js:332:16
[task 2019-03-07T13:35:33.936Z] 13:35:33 INFO - testInheritedProperty@layout/style/test/test_revert.html:65:3
[task 2019-03-07T13:35:33.937Z] 13:35:33 INFO - testProperty@layout/style/test/test_revert.html:87:5
[task 2019-03-07T13:35:33.937Z] 13:35:33 INFO - @layout/style/test/test_revert.html:101:3
[task 2019-03-07T13:35:33.938Z] 13:35:33 INFO - 103 INFO TEST-UNEXPECTED-FAIL | layout/style/test/test_revert.html | font-family: Should get the non-inherited value from somewhere (expected inheritance - inherited: sans-serif - parent: sans-serif - default: sans-serif) - didn't expect "sans-serif", but got it
[task 2019-03-07T13:35:33.938Z] 13:35:33 INFO - SimpleTest.isnot@SimpleTest/SimpleTest.js:332:16
[task 2019-03-07T13:35:33.939Z] 13:35:33 INFO - testInheritedProperty@layout/style/test/test_revert.html:71:27
[task 2019-03-07T13:35:33.939Z] 13:35:33 INFO - testProperty@layout/style/test/test_revert.html:87:5
[task 2019-03-07T13:35:33.939Z] 13:35:33 INFO - @layout/style/test/test_revert.html:101:3
[task 2019-03-07T13:35:33.940Z] 13:35:33 INFO - 104 INFO TEST-PASS | layout/style/test/test_revert.html | font-family: Should behave as if there was no author style
[task 2019-03-07T13:35:33.940Z] 13:35:33 INFO - 105 INFO TEST-PASS | layout/style/test/test_revert.html | font-feature-settings: Should inherit something non-initial to begin with
[task 2019-03-07T13:35:33.941Z] 13:35:33 INFO - 106 INFO TEST-PASS | layout/style/test/test_revert.html | font-feature-settings: Should get the non-inherited value from somewhere (expected inheritance - inherited: "liga" - parent: "liga" - default: normal)
[task 2019-03-07T13:35:33.942Z] 13:35:33 INFO - 107 INFO TEST-PASS | layout/style/test/test_revert.html | font-feature-settings: Should behave as if there was no author style
[task 2019-03-07T13:35:33.942Z] 13:35:33 INFO - 108 INFO TEST-PASS | layout/style/test/test_revert.html | font-kerning: Should inherit something non-initial to begin with
[task 2019-03-07T13:35:33.943Z] 13:35:33 INFO - 109 INFO TEST-PASS | layout/style/test/test_revert.html | font-kerning: Should get the non-inherited value from somewhere (expected inheritance - inherited: normal - parent: normal - default: auto)
[task 2019-03-07T13:35:33.943Z] 13:35:33 INFO - 110 INFO TEST-PASS | layout/style/test/test_revert.html | font-kerning: Should behave as if there was no author style
[task 2019-03-07T13:35:33.944Z] 13:35:33 INFO - 111 INFO TEST-PASS | layout/style/test/test_revert.html | font-language-override: Should inherit something non-initial to begin with
[task 2019-03-07T13:35:33.944Z] 13:35:33 INFO - 112 INFO TEST-PASS | layout/style/test/test_revert.html | font-language-override: Should get the non-inherited value from somewhere (expected inheritance - inherited: "ENG" - parent: "ENG" - default: normal)
[task 2019-03-07T13:35:33.945Z] 13:35:33 INFO - 113 INFO TEST-PASS | layout/style/test/test_revert.html | font-language-override: Should behave as if there was no author style
[task 2019-03-07T13:35:33.945Z] 13:35:33 INFO - 114 INFO TEST-PASS | layout/style/test/test_revert.html | font-size: Should inherit something non-initial to begin with
[task 2019-03-07T13:35:33.946Z] 13:35:33 INFO - 115 INFO TEST-PASS | layout/style/test/test_revert.html | font-size: Should get the non-inherited value from somewhere (expected inheritance - inherited: 18px - parent: 18px - default: 16px)
[task 2019-03-07T13:35:33.947Z] 13:35:33 INFO - 116 INFO TEST-PASS | layout/style/test/test_revert.html | font-size: Should behave as if there was no author style

This was on a "test-verify" run, on the commit that added this test (and implemented the tested feature, the CSS 'revert' keyword).

Looks like a bug in the (new) test or in the feature. emilio, could you take a look?

Blocks: 1215878
Flags: needinfo?(emilio)

Looks like the precondition of the test is failing, so not a bug in the feature, phew :)

font-family is weird, because its initial value depends both on a bunch of prefs and on the language derived from the document's charset.

So probably what's going on is that property_database.js is reading an initial font-size value, and the charset of the document is changing under the hood, causing the default style to be different. But I'm confused about how the default could change from sans-serif anyhow in our android automation.

Will send something to try to verify that theory.

Ok, so with a bit of debugging, I can see that that's the case:

https://hg.mozilla.org/try/rev/47ebada12bbe0e49df83a2cb1c2314b15545b4f8

So the default font-family changes during the test (it's different from what property_database.js computes, even though it is computed basically the same way, using the initial keyword), which doesn't make sense to me.

So, I can repro it on desktop using the following diff:

diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js
index e00677408a0f..edc619e69c30 100644
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -4571,7 +4571,7 @@ pref("gfx.font_rendering.fontconfig.max_generic_substitutions", 3);
 #endif
 #endif
 
-#if defined(ANDROID)
+// #if defined(ANDROID)
 
 pref("font.size.fixed.ar", 12);
 
@@ -4590,7 +4590,7 @@ pref("font.default.x-western", "sans-serif");
 pref("font.size.fixed.x-western", 12);
 
 # ANDROID
-#endif
+// #endif
 
 #if defined(ANDROID)
 // We use the bundled Charis SIL Compact as serif font for Firefox for Android

Ok, got it, it was way less magical, I'm surprised other tests do not suffer from this on Android. This is technically an issue introduced in bug 1511138.

Assignee: nobody → emilio
Blocks: 1511138
Flags: needinfo?(emilio)
Priority: P5 → P3

Calling getComputedStyle on an element outside of the document always returns
the empty string since bug 1511138, so we were incorrectly thinking that the
initial font-family was serif when it was not.

I chose a <meta> element instead of a <div> because they're undisplayed and they
don't cause the <body> of the document to start (though I think that only
happens from the parser).

In any case I'm pretty surprised this didn't cause issues in other tests...

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/91fcd9232c7e
Fix initial font-family detection in property-database.js. r=dholbert
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/b54eca99b0b3
followup: Remove ok() calls since we have testharness.js tests using this file.
Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.