1570759 - Consider enabling the optimization to not flush when getting fixed margins from getComputedStyle().

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Emilio Cobos Álvarez [:emilio]]

Attachment: Testcase

This would align (dubiously) our behavior with other engines, but it's not 100% clear whether that's right, see https://github.com/w3c/csswg-drafts/issues/2328.

See the following test-case for a bit of surprising behavior. If you have fixed margins, we return the "wrong" used margin, but as soon as you have a percentage margin, presumably we store the UsedMarginProperty on the frame, and return the "proper" margin.

Our behavior is inconsistent, so it's unclear what to do here...

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jwatt, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Yeah, I guess we should do this.

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1570759 - Enable the optimization to not flush for fixed margin values.

As noted this changes behavior, but it's unclear per
https://github.com/w3c/csswg-drafts/issues/2328 what behavior is correct, and
our behavior is inconsistent depending on whether there's any percentage
involved.

This matches other browsers so it's pretty low risk I'd say.

The test starts passing without changes to the test, but given the CSSWG issue I
made the test not rely on the optimization.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7fa833ff0b1c
Enable the optimization to not flush for fixed margin values. r=jwatt

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/18771 for changes under testing/web-platform/tests

Comment 6

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

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c1311fe97e4d
followup: Actually use the style value if we determine we don't need the layout one. r=bustage

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce4121497815
Don't expect assertions since they all came from margin computation.

Comment 9

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/7fa833ff0b1c
https://hg.mozilla.org/mozilla-central/rev/c1311fe97e4d
https://hg.mozilla.org/mozilla-central/rev/ce4121497815

Comment 10

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

Upstream PR merged by moz-wptsync-bot

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7fa833ff0b1c
https://hg.mozilla.org/releases/mozilla-beta/rev/c1311fe97e4d
https://hg.mozilla.org/releases/mozilla-beta/rev/ce4121497815