1708370 - New wpt failures in /html/rendering/non-replaced-elements/ [form-controls/resets.html, lists/lists-styles.html]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

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

Syncing wpt PR 28040 found new untriaged test failures in CI

Tests Affected

Firefox-only failures

/html/rendering/non-replaced-elements/form-controls/resets.html
<option> (in <select><optgroup>) - text-align: FAIL
<optgroup> - line-height: FAIL
<optgroup> - text-indent: FAIL
<option> - line-height: FAIL
<option> - text-indent: FAIL

New Tests That Don't Pass

/html/rendering/non-replaced-elements/lists/lists-styles.html
<ol type="none"> - list-style-type: FAIL (Chrome: PASS)
<li type="none"> - list-style-type: FAIL (Chrome: PASS)
<ol type="NONE"> - list-style-type: FAIL (Chrome: PASS)
<li type="NONE"> - list-style-type: FAIL (Chrome: PASS)
<ol type="disc"> - list-style-type: FAIL (Chrome: PASS)
<ol type="DISC"> - list-style-type: FAIL (Chrome: PASS)
<ol type="circle"> - list-style-type: FAIL (Chrome: PASS)
<ol type="CIRCLE"> - list-style-type: FAIL (Chrome: PASS)
<ol type="square"> - list-style-type: FAIL (Chrome: PASS)
<ol type="SQUARE"> - list-style-type: FAIL (Chrome: PASS)

CI Results

Gecko CI (Treeherder)
GitHub PR Head

Notes

These updates will be on mozilla-central once bug 1708171 lands.

Note: this bug is for tracking fixing the issues and is not
owned by the wpt sync bot.

This bug is linked to the relevant tests by an annotation in
https://github.com/web-platform-tests/wpt-metadata. These annotations
can be edited using the wpt interop dashboard
https://jgraham.github.io/wptdash/

If this bug is split into multiple bugs, please also update the
annotations, otherwise we are unable to track which wpt issues are
already triaged. Resolving as duplicate or closing this issue should
be cause the bot to automatically update or remove the annotation.

Comment 1

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Web Platform Test Sync Bot (Matrix: #interop:mozilla.org) from comment #0)

Syncing wpt PR 28040 found new untriaged test failures in CI

Tests Affected

Firefox-only failures

/html/rendering/non-replaced-elements/form-controls/resets.html
<option> (in <select><optgroup>) - text-align: FAIL
<optgroup> - line-height: FAIL
<optgroup> - text-indent: FAIL
<option> - line-height: FAIL
<option> - text-indent: FAIL

The above still fail ^^^

New Tests That Don't Pass

/html/rendering/non-replaced-elements/lists/lists-styles.html

These two sub-tests from the original report still fail in Fx 112:
<li type="none"> - list-style-type: FAIL (Chrome: PASS)
<li type="NONE"> - list-style-type: FAIL (Chrome: PASS)

Others below were fixed:

<ol type="none"> - list-style-type: FAIL (Chrome: PASS)

Fixed

<ol type="NONE"> - list-style-type: FAIL (Chrome: PASS)

Fixed

<ol type="disc"> - list-style-type: FAIL (Chrome: PASS)

Fixed

<ol type="DISC"> - list-style-type: FAIL (Chrome: PASS)

Fixed

<ol type="circle"> - list-style-type: FAIL (Chrome: PASS)

fixed

<ol type="CIRCLE"> - list-style-type: FAIL (Chrome: PASS)

fixed

<ol type="square"> - list-style-type: FAIL (Chrome: PASS)

fixed

<ol type="SQUARE"> - list-style-type: FAIL (Chrome: PASS)

fixed

Comment 2

[Hsin-Yi Tsai (she/her) [:hsinyi]]

These interop-2023-forms tests still fail:
<option> (in <select><optgroup>) - text-align: FAIL
<optgroup> - line-height: FAIL
<optgroup> - text-indent: FAIL
<option> - line-height: FAIL
<option> - text-indent: FAIL

Hi Emilio, I recalled when we talked about these tests a while ago on Matrix, the effort here would either fixing the buggy tests or some tweak in forms.css. I plan to assign this to our team's student worker, Vincent. If you have more pointers for him to start from, it's highly appreciated. :)

Comment 3

[Emilio Cobos Álvarez (:emilio)]

So it seems some of the styles in here are not per spec.

It seems line-height comes from bug 82265, text-indent comes from bug 56253. We should check whether we can remove them (specially after bug 1744009 I think we probably can), but we should test <select> and <select multiple> with various style combinations to confirm rendering matches other browsers.

Comment 4

[Vincent Hilla [:vhilla]]

Thanks for the pointers. After correcting the styles in forms.css, optgroup is still failing for line-height, as it's not inheriting the style. line-height is an inherited property though and works correctly on other elements.

<div id=container style="line-height: 5px">
  <optgroup id=ogt></optgroup>
</div>
<script>
console.log(getComputedStyle(ogt).getPropertyValue("line-height"));
</script>

This writes normal to console.

:emilio, do you know what could cause this?

Comment 5

[Emilio Cobos Álvarez (:emilio)]

The font shorthand also resets line-height per spec, so it's being reset by font: -moz-list. You can see this easily if you enable "Show browser styles" in the devtools settings fwiw :)

Comment 6

[Vincent Hilla [:vhilla]]

Alright, so far it looks good :) Two more things.

Bug 82265, Bug 56253 and Bug 645642 added line-height, text-indent and text-align for more than the elements relevant to this bug. Should I look into removing these rules from other elements too, if it is not part of the HTML spec?

And looking at this test from Bug 82265, the optgroup is rendered differently in Firefox vs Blink, WebKit, when no label is provided. The reason is that Firefox uses a ::before for the label that has zero height for an empty text. The specification is ambiguous on this. Should I

• file a spec issue for that
• file a bug with Firefox for using a ::before / having zero height?

Comment 7

[Emilio Cobos Álvarez (:emilio)]

(In reply to Vincent Hilla [:vhilla] from comment #6)

Bug 82265, Bug 56253 and Bug 645642 added line-height, text-indent and text-align for more than the elements relevant to this bug. Should I look into removing these rules from other elements too, if it is not part of the HTML spec?

Possibly, but I'd do it in separate bugs or at least separate patches to be able to back out easily if there are issues.

And looking at this test from Bug 82265, the optgroup is rendered differently in Firefox vs Blink, WebKit, when no label is provided. The reason is that Firefox uses a ::before for the label that has zero height for an empty text. The specification is ambiguous on this. Should I

• file a spec issue for that
• file a bug with Firefox for using a ::before / having zero height?

If you change the optgroup:before rule to have content: "\200b" attr(label), does it work then?

Comment 8

[Vincent Hilla [:vhilla]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)

If you change the optgroup:before rule to have content: "\200b" attr(label), does it work then?

Sure does. It does seem wrong to me though to use ::before for this.

Comment 9

[Vincent Hilla [:vhilla]]

Attachment: Bug 1708370 - adjust UA stylesheet to match spec for form rendering. r=emilio

Comment 10

[Emilio Cobos Álvarez (:emilio)]

(In reply to Vincent Hilla [:vhilla] from comment #8)

Sure does. It does seem wrong to me though to use ::before for this.

It is not great to use generated content for this indeed, but I don't think changing it should be in scope for this bug.

Separate bug is fine, but also unless it actively creates compat issues or unless the spec defines it properly in a way that it's interoperable, I don't think changing if just for the sake of changing it is worth it.

Comment 11

[Vincent Hilla [:vhilla]]

optgroup::before is tracked in Bug 1498358

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/24043b03134b
adjust UA stylesheet to match spec for form rendering. r=emilio

Comment 13

[Cristina Horotan [:chorotan]]

Backed out for causing mochitest failures at test_HTMLSpec.html and browser_selectables.js

Backout: https://hg.mozilla.org/integration/autoland/rev/7e79b3d4ef09df854ad0571376054a7dafef01d9

Failure push: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=CP4o8FV3SWSRev6TMW9gBw.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=24043b03134b1ee572d64e88dd33206591195e51

Failure logs:
https://treeherder.mozilla.org/logviewer?job_id=419855790&repo=autoland&lineNumber=3208
https://treeherder.mozilla.org/logviewer?job_id=419855440&repo=autoland&lineNumber=16479

Comment 14

[Vincent Hilla [:vhilla]]

:emilio (In reply to Emilio Cobos Álvarez (:emilio) from comment #7)

If you change the optgroup:before rule to have content: "\200b" attr(label), does it work then?

this breaks test_HTMLSpec.html and browser_selectables.js, as another staticText is inserted within the accessibility information.

I can update the expectation data, but do you think an additional empty staticText before any grouplabel is an accessibility problem? If so, we could also accept for now that empty label means the label will have zero height.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

301 Jamie, but my guess is that the extra zwsp is harmless.

Comment 16

[James Teh [:Jamie]]

It's probably harmless, but ugly/slightly confusing, since it's visible to screen reader review cursors and inspection tools. Most users probably won't see it, but some might. I think it's acceptable for now if that's the cleanest solution.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a5a58f2c2915
adjust UA stylesheet to match spec for form rendering. r=emilio

Comment 18

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/a5a58f2c2915