Skip to content

LibWeb: Don't inflate margin-right for over-constrained blocks#8156

Open
wakamex wants to merge 2 commits intoLadybirdBrowser:masterfrom
wakamex:fix-overconstrained-margin-right
Open

LibWeb: Don't inflate margin-right for over-constrained blocks#8156
wakamex wants to merge 2 commits intoLadybirdBrowser:masterfrom
wakamex:fix-overconstrained-margin-right

Conversation

@wakamex
Copy link

@wakamex wakamex commented Feb 25, 2026

getComputedStyle().marginRight returns an inflated value for over-constrained blocks (e.g. 400px instead of 0px), disagreeing with Chrome, Firefox, and Safari.

When a block's width, margin-left, and margin-right are all explicitly set and don't fill the containing block, CSS 2.2 § 10.3.3 says to inflate margin-right to satisfy the constraint equation. Since a706d0e switched getComputedStyle() to return used values for margins, this inflated value now leaks to the web. All major browsers store the declared value instead — see CSSWG #2328.

The inflated value serves no layout purpose: position is determined by margin-left, width is already set, and siblings position independently.

Reproducer (paste in DevTools console on any page):

let d = document.createElement('div');
d.style.cssText = 'width:100px; margin:0px';
document.body.appendChild(d);
getComputedStyle(d).marginRight;
// Ladybird: "700px" — Chrome/Firefox/Safari: "0px"

First commit adds a failing test, second commit applies the fix and rebaselines 365 layout test expectations.

One WPT test flips from fail to pass in css/css-logical/inheritance.html. The assert_not_inherited('margin-inline-end', '0px', '10px') now passes because the parent's margin-inline-end no longer gets inflated as it's an over-constrained case.

One WPT ref test is a genuine regression, as the existing "correct" solution relied on inflation: floats-wrap-bfc-outside-001.xht.

Fixes #4729
Supersedes #6606

@ladybird-bot
Copy link
Collaborator

Hello!

One or more of the commit messages in this PR do not match the Ladybird code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why.
Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

@wakamex wakamex force-pushed the fix-overconstrained-margin-right branch from 21c2a59 to f4d3b98 Compare February 25, 2026 05:22
@wakamex wakamex force-pushed the fix-overconstrained-margin-right branch from f4d3b98 to 2fb3fbf Compare February 25, 2026 16:06
gmta
gmta previously approved these changes Feb 27, 2026
@gmta gmta dismissed their stale review February 27, 2026 09:25

One outstanding issue

@wakamex wakamex force-pushed the fix-overconstrained-margin-right branch from 638e56f to 875b0e1 Compare February 27, 2026 14:47
@gmta gmta enabled auto-merge (rebase) February 27, 2026 14:49
@gmta gmta disabled auto-merge February 27, 2026 15:51
@gmta
Copy link
Collaborator

gmta commented Feb 27, 2026

@wakamex multiple tests are failing. Please check the test results locally and see if they need updating.

@github-actions github-actions bot added the conflicts Pull request has merge conflicts that need resolution label Feb 27, 2026
@github-actions
Copy link

Your pull request has conflicts that need to be resolved before it can be reviewed and merged. Make sure to rebase your branch on top of the latest master.

@wakamex wakamex force-pushed the fix-overconstrained-margin-right branch from 875b0e1 to c131700 Compare February 27, 2026 22:43
wakamex and others added 2 commits February 27, 2026 17:44
This test currently fails: getComputedStyle().marginRight returns
an inflated value (e.g. 400px instead of 0px) for blocks where
width and both margins are explicitly set but don't fill the
containing block.

All other browsers return the declared value. See
w3c/csswg-drafts#2328.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CSS 2.2 § 10.3.3 says to adjust margin-right to satisfy the
constraint equation when a block's width and both margins are
explicitly set. However, the inflated value serves no layout
purpose (position is determined by margin-left, width is already
set, siblings position independently) and all major browsers
agree on not storing it.

Since a706d0e switched getComputedStyle() to return used
values for margins, this inflated margin-right now leaks through
to the web via getComputedStyle().marginRight, disagreeing with
Chrome, Firefox, and Safari.
See w3c/csswg-drafts#2328.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@wakamex wakamex force-pushed the fix-overconstrained-margin-right branch from c131700 to 5908b2c Compare February 27, 2026 22:48
@github-actions github-actions bot removed the conflicts Pull request has merge conflicts that need resolution label Feb 27, 2026
@wakamex
Copy link
Author

wakamex commented Feb 27, 2026

Fixed 2 of the 3 failing tests. 1 was a missing --rebalance due to rebase. 2 is a WPT test that flips from fail to pass. 3 is a real regression that relied on the inflated margin. I'll have to take a closer look at that one. latter 2 I documented in the PR body as such:

One WPT test flips from fail to pass in css/css-logical/inheritance.html. The assert_not_inherited('margin-inline-end', '0px', '10px') now passes because the parent's margin-inline-end no longer gets inflated as it's an over-constrained case.

One WPT ref test is a genuine regression, as the existing "correct" solution relied on inflation: floats-wrap-bfc-outside-001.xht.

also found a material change in the last --rebaseline that needs fixing (gist)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected used value of margin-right

3 participants