Closed Bug 1613183 Opened 5 years ago Closed 5 years ago

[wpt-sync] Sync PR 21578 - [css-pseudo] Fix legacy ::marker margins not getting updated

Categories

(Core :: Layout: Generated Content, Lists, and Counters, task, P4)

task

Tracking

()

RESOLVED FIXED
mozilla75
Tracking Status
firefox75 --- fixed

People

(Reporter: mozilla.org, Unassigned)

References

()

Details

(Whiteboard: [wptsync downstream])

Sync web-platform-tests PR 21578 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/web-platform-tests/wpt/pull/21578
Details from upstream follow.

Oriol Brufau <obrufau@igalia.com> wrote:

[css-pseudo] Fix legacy ::marker margins not getting updated

Outside markers behave out-of-flowish, so their intrinsic contribution
to the list item should be 0.

In legacy, this is done by adding negative margins, so that the width of
the marker plus its margins are 0.

However, when calculating the intrinsic contributions of the children,
the code uses the margins in the computed style, not the margins of the
box. Therefore, the marker code used to replace the computed style with
another one that had the changed margins.

This approach was error-prone, for example special care was needed to
reapply the previously calculated margins when the computed style
changed, in order to avoid triggering unnecessary layout.

However, this caused a problem: if 'list-style-position' changed, the
old margins were preserved. But the margins depend on whether the marker
is inside or outside!

This patch fixes the problem and stops changing the computed style of
the marker.

For inside markers, margins don't depend on the size of the marker, and
they were already applied to the computed style in style_adjuster.cc, so
they work well. For outside markers, the patch skips them when
calculating the intrinsic size of the list item.

Bug: 1048672, 457718

TEST=external/wpt/css/css-pseudo/marker-intrinsic-contribution-001.html
TEST=external/wpt/css/css-pseudo/marker-intrinsic-contribution-002.html

The former test fails in LayoutNG because trailing marker spaces are not
preserved, and fails in legacy due to the lack of 'content' support.

Change-Id: Ic21603e9fe1b17b11237286dac3f207b1837a07e
Reviewed-on: https://chromium-review.googlesource.com/2035962
WPT-Export-Revision: e64f98eeee6f576f75475c6a670d9c1126dec0db

Component: web-platform-tests → Layout: Generated Content, Lists, and Counters
Product: Testing → Core

CI Results

Ran 13 Firefox configurations based on mozilla-central, and Firefox, Chrome, and Safari on GitHub CI

Total 45 tests

Status Summary

Firefox

OK : 1
PASS: 9[GitHub] 52[Gecko-android-em-7.0-x86_64-debug-geckoview, Gecko-android-em-7.0-x86_64-opt-geckoview, Gecko-linux1804-64-asan-opt, Gecko-linux1804-64-debug, Gecko-linux1804-64-opt, Gecko-linux1804-64-qr-debug, Gecko-linux1804-64-qr-opt, Gecko-windows10-64-debug, Gecko-windows10-64-opt, Gecko-windows10-64-qr-debug, Gecko-windows10-64-qr-opt, Gecko-windows7-32-debug, Gecko-windows7-32-opt]

Chrome

OK : 1
PASS: 8
FAIL: 1

Safari

OK : 1
PASS: 6
FAIL: 3

Links

Gecko CI (Treeherder)
GitHub PR Head
GitHub PR Base

Pushed by wptsync@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0d0c5aef21dc [wpt PR 21578] - [css-pseudo] Fix legacy ::marker margins not getting updated, a=testonly
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla75
You need to log in before you can comment on or make changes to this bug.