1841318 - WPT failures in css/css-grid/grid-model/grid-[inline-]first-[letter|line]-003.html

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Thomas Wisniewski [:twisniewski]]

wpt.fyi:

• https://wpt.fyi/results/css/css-grid/grid-model/grid-first-letter-003.html
• https://wpt.fyi/results/css/css-grid/grid-model/grid-first-line-003.html
• https://wpt.fyi/results/css/css-grid/grid-model/grid-inline-first-letter-003.html
• https://wpt.fyi/results/css/css-grid/grid-model/grid-inline-first-line-003.html

If I'm not mistaken, these all seem related to margins collapsing properly on :first-letter and :first-line within grids, so they appear to be similar.

These failures may very well be directly related to bug 1841317.

Comment 1

[Daniel Holbert [:dholbert]]

(In reply to Thomas Wisniewski [:twisniewski] from comment #0)

If I'm not mistaken, these all seem related to margins collapsing properly on :first-letter and :first-line within grids, so they appear to be similar.

Looking at the screenshots, it's actually the color that we're getting wrong, not anything about collapsing or layout.

These failures may very well be directly related to bug 1841317.

Indeed, I think this is the thing I described in bug 1841317 comment 1. The testcases in this bug here all have a grid item with ::first-letter or ::first-line styling, whose content is all nested inside of a p element, and we're failing to dig into the p element when finding the appropriate thing to style for ::first-{letter,line}.

Comment 2

[Daniel Holbert [:dholbert]]

Hmm, I'm going to un-dupe actually; I think the tests in bug 1841317 are about checking that :first-line skips past grid children when finding the first line (which is a subtle thing), whereas this bug here is about whether or not the first-line applies in a grid item (which is less subtle).

Per bug 1841317 comment 7, I think the failures here are due to an unrelated-to-grid bug; they're accidentally relying on blocks discovering their first-line in a descendant, which doesn't work properly for us right now and is tracked in bug 317081. We can fix the tests so they don't depend on that; there's separate and more-direct WPT test coverage for this behavior in these other tests at least:
https://wpt.fyi/results/css/css-pseudo/first-line-on-ancestor-block.html
https://wpt.fyi/results/css/css-pseudo/first-line-change-inline-color.html

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: Bug 1841318: Remove unnecessary div wrapper from some grid ::first-letter/::first-line WPTs, to avoid tripping over an unrelated-to-grid Firefox bug. r?emilio

These tests are trying to validate that the first-line and first-letter pseudos
match properly when specified on a grid item, but these tests were testing this
slightly awkwardly by checking whether these pseudos can change the color of
text that happens to live in a descendant of that grid item.

That descendant-targeting doesn't work in Firefox right now, due to a
longstanding bug with these pseudos[1]. That bug has nothing to do with grid
or grid items, however.

There's no reason that these tests need to be targeting a descendant to
validate that these pseudos match. We can just adjust the test to remove the
unnecessary extra wrapper, which then lets us validate more directly that they
do in fact properly change the color of the first letter or first line directly
in that grid item.

(Note that we do have other WPT coverage[2][3] for the known Firefox
pseudo-matching-text-in-descendants bug, as well.)

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=317081
[2] https://wpt.fyi/results/css/css-pseudo/first-line-on-ancestor-block.html
[3] https://wpt.fyi/results/css/css-pseudo/first-line-change-inline-color.html

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7df10ffc0357
Remove unnecessary div wrapper from some grid ::first-letter/::first-line WPTs, to avoid tripping over an unrelated-to-grid Firefox bug. r=emilio

Comment 5

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/41677 for changes under testing/web-platform/tests

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/7df10ffc0357

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot