Closed Bug 1588366 Opened 5 years ago Closed 5 years ago

[wpt-sync] Sync PR 19660 - [LayoutNG] Fix NGLineBreaker not to hang on negative margins

Categories

(Core :: CSS Parsing and Computation, task, P4)

task

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: mozilla.org, Unassigned)

References

()

Details

(Whiteboard: [wptsync downstream])

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

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

Koji Ishii <kojii@chromium.org> wrote:

[LayoutNG] Fix NGLineBreaker not to hang on negative margins

This patch fixes a case where |NGLineBreaker| loops
infinitely when there is a negative margin. The case was a
regression by r701601 (crrev.com/c/1826063).

Handling combinations of a) trailable items, b) empty spans,
and c) negative margins together is getting complex. This
patch revises how we handle them.

One of the complexities is open tags being ambiguous; e.g.,
this open tag is not trailable and thus break before it:
\<span>text
but these are trailable:
\<span> text
\<span>\</span>text
\<span> \</span>text

Before this patch, |NGLineBreaker| tries to stop as early as
possible, rewind, and consume trailable items.

The new approach is to try to consume possibly trailable
items before it rewinds. Because consume is forward-only but
rewind has full access to items before and after, rewind can
determine ambiguous trailable items using context.
blink_perf is mostly neutral for this change.
https://pinpoint-dot-chromeperf.appspot.com/job/11fdf216c20000

Case 2 and 5 in the test fail in legacy, because the legacy
behavior is based on a bug (crbug.com/979894) or the new
behavior makes more sense, and the new behavior is
interoperable with Edge/Gecko.

Note, this change should be able to eliminate |kTrailable|
state, but we still need it in a few places. It is for future
patches to clean them up.

Bug: 1011816
Change-Id: Ic915a26f1ee570ebe999ffbeee680659dcbd9789

Reviewed-on: https://chromium-review.googlesource.com/1849737
WPT-Export-Revision: 9af3f935db608bf0bcbc4fa3c230f075028886bb

Component: web-platform-tests → CSS Parsing and Computation
Product: Testing → Core
## GitHub CI Results
wpt.fyi [PR Results](https://wpt.fyi/results/?sha=8816c23fd4bbd3037d477782d9eb16f3dd319020&label=pr_head) [Base Results](https://wpt.fyi/results/?sha=8816c23fd4bbd3037d477782d9eb16f3dd319020&label=pr_base)

Ran 1 tests and 8 subtests

### Firefox
  OK     : 1
  PASS   : 7

### Chrome
  OK     : 1
  PASS   : 6
  FAIL   : 1

### Safari
  OK     : 1
  PASS   : 5
  FAIL   : 2

### Existing tests that now have a worse result

/css/CSS2/linebox/inline-negative-margin-001.html
   #container 1: Firefox: PASS->MISSING, Chrome: PASS->MISSING, Safari: PASS->MISSING
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e9bce80c7ad
[wpt PR 19660] - [LayoutNG] Fix NGLineBreaker not to hang on negative margins, a=testonly
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
You need to log in before you can comment on or make changes to this bug.