Adding web-platform tests ref-tests for text-underline-offset
Categories
(Core :: Layout, enhancement, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox70 | --- | fixed |
People
(Reporter: cmarlow, Assigned: cmarlow)
References
Details
Attachments
(1 file, 1 obsolete file)
|
Assignee | |
Comment 1•5 years ago
|
||
|
||
Updated•5 years ago
|
|
|
Assignee | |
Comment 2•5 years ago
|
||
|
|
||
Updated•5 years ago
|
|
||
Comment 3•5 years ago
|
||
Pretty sure this needs to wait until bug 1561657 has been addressed (per the recent patch-update to invert the sign in some tests here).
Adding bug-dependency as a reminder / to make that explicit.
Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3fd103a9fce8 Added reftests for text-underline-offset r=dholbert
|
||
Comment 5•5 years ago
|
||
Backed out 3 changesets (Bug 1561657, Bug 1557857, Bug 1562704) for causing Wr permafailures in z:/build/build/src/layout/base/nsCSSFrameConstructor.cpp:3962 CLOSED TREE
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=255307738&repo=autoland&lineNumber=32915
|
|
||
Comment 6•5 years ago
|
||
This patch is tests-only, so it's just uncovering an until-now-undiscovered issue, it seems.
Specifically, this patch's new test text-underline-offset-scroll-001.html , here in the commit, is triggering this fatal assertion:
Assertion failure: cachedStyles[i]->EqualForCachedAnonymousContentStyle(*cs)
(cached anonymous content styles should be identical to those we would compute normally),
at z:/build/build/src/layout/base/nsCSSFrameConstructor.cpp:3962
This is an assertion that was added quite recently in bug 1554571 part 3.
It looks like we have bug 1562359 filed on one way that we know of to trigger that assertion. Not sure yet if this test is the same as that bug or a different issue.
|
|
||
Comment 7•5 years ago
|
||
heycam, do you know why this web platform test would be causing this assertion failure (only on Windows 10), and do you have a suggestion for what Charlie can do to fix or work around it? And is this the same issue as bug 1562359?
For example: would it be sane to mark the test as disabled on Windows for now, so that we can land these tests for text-underline-offset without having to be blocked on the assertion-failure? (e.g. with a reference to bug 1562359 alongside the disabling in the .ini file)
|
||
Comment 8•5 years ago
|
||
So this is because we enable the pref at runtime, which changes what the all property expands to, but we don't reparse UA stylesheets. This is quite an annoying problem looks like...
This code restrict the expanded properties to the ones enabled for all content, which does the pref check. We may want to fix this making it origin-aware, maybe, but that looks a bit too annoying since we don't have that information from everywhere AFAIK.
|
|
Assignee | |
Comment 9•5 years ago
|
||
Fixed it by separating the text styling into a span, avoiding issues on Windows with scrollable divs since the div's style that was cached before turning text-underline-offset pref on did not match the style that was applied once the pref was flipped on.
|
|
||
Comment 10•5 years ago
|
||
This fix (comment 9) seems to avoid the assertion-failure on my Win10 debug build.
For folks who want to investigate the assertion-failure (heycam/emilio?), e.g. over on bug 1562359: you'll probably be able to reliably trigger it (on Windows at least) by adjusting the soon-to-be-landed WPT test text-underline-offset-scroll-001.html such that its text-decoration styling is all on the scrollable div, and then running
./mach wpt --headless testing/web-platform/tests/css/css-text-decor/text-underline-offset-scroll-001.html
|
|
||
Comment 11•5 years ago
|
||
That may not work on its own, it may depend on whether the test before had that pref enabled or not. As I said I can see how this bug can happen (comment 8), I'd like to check with Cam what's the best way to fix this.
Cam, do you have any opinion on comment 8?
|
|
||
Comment 12•5 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #11)
That may not work on its own, it may depend on whether the test before had that pref enabled or not.
(this was referring to my ./mach wpt command from comment 10, where "work" = "trigger the assertion")
It does seem to work (trigger the assertion) on its own, FWIW (in my local debug build on Win10 with Charlie's original patch that landed here). Apparently we do enough spin-up work before this test's dynamic pref-flip to get the styles cached, even in a single-test WPT run.
|
||
Comment 13•5 years ago
|
||
I am glad I added the assertions. :-) Making the all shorthand include properties that are disabled for content sounds like the right thing to do. I will see how hard it is -- filed bug 1564597.
|
||
Comment 14•5 years ago
|
||
Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ed32865ed737 Added reftests for text-underline-offset r=dholbert
|
||
Comment 15•5 years ago
|
||
| bugherder | ||
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/17767 for changes under testing/web-platform/tests
Upstream PR merged
Description
•