layout: Stop splitting inline boxes when they contain block-levels#41492
layout: Stop splitting inline boxes when they contain block-levels#41492Loirooriol merged 1 commit intoservo:mainfrom
Conversation
|
🔨 Triggering try run (#20455925360) for Linux (WPT) |
|
🛠 These changes could not be applied onto the latest upstream WPT. Servo's copy of the Web Platform Tests may be out of sync. |
|
Test results for linux-wpt from try job (#20455925360): Flaky unexpected result (28)
Stable unexpected results that are known to be intermittent (31)
|
|
✨ Try run (#20455925360) succeeded. |
|
This seems to be relevant to #41275 as well. |
|
🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#56903) with upstreamable changes. |
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#56903). |
|
🔨 Triggering try run (#20513122554) for Linux (WPT) |
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#56903). |
c401604 to
73d79ef
Compare
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#56903). |
1 similar comment
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#56903). |
|
Test results for linux-wpt from try job (#20513122554): Flaky unexpected result (42)
Stable unexpected results that are known to be intermittent (26)
|
|
✨ Try run (#20513122554) succeeded. |
|
🤖 This change no longer contains upstreamable changes to WPT; closed existing upstream pull request (web-platform-tests/wpt#56903). |
mrobinson
left a comment
There was a problem hiding this comment.
This is such a wonderful simplification and improvement. I'm very excited for it.
| let job = BlockLevelJob { | ||
| info: info.clone(), | ||
| box_slot, | ||
| kind, | ||
| propagated_data, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
I think you can move this below the conditional to avoid allocating it in the case that you do an early return.
There was a problem hiding this comment.
It's used inside the conditional, it can't be avoided. The conditional is just to decide whether it goes to the inline formatting context or not.
We now follow the same approach as Blink: sequences of block-levels in an inline formatting context get wrapped in an anonymous block, which is treated as a line item. Inline ancestors are no longer split. This means that inline elements will now generate a single inline box, and the box tree makes more sense in general. This will help for incremental layout. This also means that block-level elements will now be properly affected by efects like opacity of filters set on inline ancestors. Recently, WebKit has done some similar work, but without the anonymous blocks. We might also consider removing them in the future. Google's explainer: https://docs.google.com/document/d/15kgdIHhb9EVNup6Ir5NWwJxpzY5GH0ED7Ld3iMW3HlA/ Signed-off-by: Oriol Brufau <[email protected]>
|
BTW, I tried forcing all block containers to have an inline formatting context, and placing block-levels there. There were about 1000 tests failing, so this patch is not perfect. But I think it's fine to land as-is and refine in follow-ups (adding tests!). |
…#43586) When encountering a block-level inside an inline box, we would wrap it (together with other block-level siblings, if any) inside an anonymous block box. That was complicating the logic for no real benefit, so just get rid of these anonymous wrappers, and allow block-levels as direct children of inlines. This aligns Servo with WebKit, which did the same refactoring: https://commits.webkit.org/304357@main Blink still generates these wrappers, but its design doc acknowledges that "the anonymous box is not strictly required". Testing: No existing test fails. But note that, while minimal, this may have some observable implications. For example, as an accidental side-effect of #41492, Servo aligned with the CSSWG resolution in w3c/csswg-drafts#11462, but now we will revert to the previous behavior. I will address it in a follow-up, and add a test. Fixes: #41636 Signed-off-by: Oriol Brufau <[email protected]>
We now follow the same approach as Blink: sequences of block-levels in an inline formatting context get wrapped in an anonymous block, which is treated as a line item. Inline ancestors are no longer split.
This means that inline elements will now generate a single inline box, and the box tree makes more sense in general. This will help for incremental layout.
This also means that block-level elements will now be properly affected by effects like opacity of filters set on inline ancestors.
Recently, WebKit has done some similar work, but without the anonymous blocks. We might also consider removing them in the future.
Google's explainer: https://docs.google.com/document/d/15kgdIHhb9EVNup6Ir5NWwJxpzY5GH0ED7Ld3iMW3HlA/
Testing: Several tests are now passing
Fixes: #39813