1846853 - Negative outline-offset is not clamped as spec

Status: RESOLVED FIXED

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

Description

[Oliver Medhurst [:canadahonk]]

Attachment: Outline test file (for repro)

As per https://www.w3.org/TR/css-ui-4/#outline-offset (last paragraph):

Negative values must cause the outline to shrink into the border box. Both the height and the width of the outside of the shape drawn by the outline should not become smaller than twice the computed value of the outline-width property to make sure that an outline can be rendered even with large negative values. User agents should apply this constraint independently in each dimension. If the outline is drawn as multiple disconnected shapes, this constraint applies to each shape separately.

Currently Chromium does this but we do not (/ it's bugged?); too small a outline-offset value causes the outline to not be shown.

Comment 1

[Oliver Medhurst [:canadahonk]]

Attachment: Bug 1846853 - Clamp negative outline-offset per spec

A negative outline-offset value should be clamped per spec as to stop it
from becoming invisible with large negative values.

Spec ref: https://www.w3.org/TR/css-ui-4/#outline-offset

WIP. Todo:

• Write WPT reftest

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/959c2f0abc19
Clamp negative outline-offset per spec r=emilio

Comment 3

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

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

Comment 4

[Narcis Beleuzu [:NarcisB]]

Backed out for reftest failures on outline-013.html & 424236-5.html

Backout link: https://hg.mozilla.org/integration/autoland/rev/d44458b9c71f8290cf9c3c4344097d738a674a44
Log link of outline-013.html: https://treeherder.mozilla.org/logviewer?job_id=426781064&repo=autoland&lineNumber=12841
Log link of 424236-5.html : https://treeherder.mozilla.org/logviewer?job_id=426781167&repo=autoland&lineNumber=8120

Comment 5

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

Upstream PR was closed without merging

Comment 6

[Oliver Medhurst [:canadahonk]]

Updated WPT expectations and removed that old (16 years) layout reftest, will wait for rereview before pushing again.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/337531b04603
Clamp negative outline-offset per spec r=emilio

Comment 8

[Serban Stanca [:SerbanS]]

Backed out for causing reftests failures.

• Backout link
• Push with failures
• Failure Log
• Failure line: REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/bugs/424236-9.html == layout/reftests/bugs/424236-9-ref.html | image comparison, max difference: 255, number of differing pixels: 13

Comment 9

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

Upstream PR was closed without merging

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5678a26be6d4
Clamp negative outline-offset per spec r=emilio

Comment 11

[Oliver Medhurst [:canadahonk]]

Another old reftest was bugged, fixed and repushed.

Comment 12

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/5678a26be6d4

Comment 13

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

Upstream PR merged by moz-wptsync-bot

Comment 14

[Andra Esanu (needinfo me)]

(In reply to Pulsebot from comment #10)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5678a26be6d4
Clamp negative outline-offset per spec r=emilio

== Change summary for alert #39391 (as of Mon, 28 Aug 2023 05:52:50 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
4%	cnn FirstVisualChange	windows10-64-shippable-qr	fission warm webrender	468.36 -> 448.32
3%	cnn SpeedIndex	windows10-64-shippable-qr	fission warm webrender	547.41 -> 530.50
2%	cnn ContentfulSpeedIndex	windows10-64-shippable-qr	fission warm webrender	565.40 -> 551.96

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=39391

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Fairly skeptic this change is the culprit. Positive cnn speed improvements were noticed in try runs for bug 1849720. Could that be the case?

Comment 16

[Alexandru Ionescu (needinfo me) [:alexandrui]]

Hi :emilio. The push you reference has some fails (builds probably). Can we have the push you seen cnn speed improvements, please?

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Sure, here

Comment 18

[Alexandru Ionescu (needinfo me) [:alexandrui]]

Thanks!