mirror of
https://github.com/servo/servo.git
synced 2025-09-18 19:08:22 +01:00
layout: Fix propagation of overflow to viewport (#39173)
This patch refactors the logic for propagating overflow to the viewport, fixing various issues: - Now we won't propagate from the root element if it has no box. Note the fix isn't observable in Servo because we lack scrollbars. - If the first `<body>` element has no box, we won't keep searching for other `<body>` elements. This deviates from the spec, but aligns us with other browsers. - We won't propagate from the `<body>` if it has no box. We were already handling `display: none` but not `display: contents`. This deviates from the spec, but aligns us with other browsers. Also, when we flag the root or `<body>` as having propagated `overflow` to the viewport, we retrieve the `LayoutBoxBase`. Therefore, now we get the computed style from the `LayoutBoxBase` in a single operation, instead of first retrieving the style from the DOM element and then getting the `LayoutBoxBase` from the box. Testing: Adding more tests. We were only failing one of them, but it's hard to test the fixes given that we don't show scrollbars. The tests that were already passing are useful too, e.g. Firefox fails one of them. Signed-off-by: Oriol Brufau <obrufau@igalia.com>
This commit is contained in:
Oriol Brufau
GitHub
parent
22fbb3458b
commit
4451ce0ef1
12 changed files with 296 additions and 69 deletions
|
|
@ -170,24 +170,28 @@ impl LayoutBox {
|
|||
}
|
||||
}
|
||||
|
||||
pub(crate) fn with_base_mut(&mut self, callback: impl Fn(&mut LayoutBoxBase)) {
|
||||
pub(crate) fn with_base_mut_fold<T>(
|
||||
&mut self,
|
||||
init: T,
|
||||
callback: impl Fn(T, &mut LayoutBoxBase) -> T,
|
||||
) -> T {
|
||||
match self {
|
||||
LayoutBox::DisplayContents(..) => {},
|
||||
LayoutBox::BlockLevel(block_level_box) => {
|
||||
block_level_box.borrow_mut().with_base_mut(callback);
|
||||
},
|
||||
LayoutBox::InlineLevel(inline_items) => {
|
||||
for inline_item in inline_items {
|
||||
inline_item.borrow_mut().with_base_mut(&callback);
|
||||
}
|
||||
},
|
||||
LayoutBox::FlexLevel(flex_level_box) => {
|
||||
flex_level_box.borrow_mut().with_base_mut(callback)
|
||||
},
|
||||
LayoutBox::TableLevelBox(table_level_box) => table_level_box.with_base_mut(callback),
|
||||
LayoutBox::TaffyItemBox(taffy_item_box) => {
|
||||
taffy_item_box.borrow_mut().with_base_mut(callback)
|
||||
LayoutBox::DisplayContents(..) => init,
|
||||
LayoutBox::BlockLevel(block_level_box) => block_level_box
|
||||
.borrow_mut()
|
||||
.with_base_mut(|base| callback(init, base)),
|
||||
LayoutBox::InlineLevel(inline_items) => inline_items.iter().fold(init, |acc, item| {
|
||||
item.borrow_mut().with_base_mut(|base| callback(acc, base))
|
||||
}),
|
||||
LayoutBox::FlexLevel(flex_level_box) => flex_level_box
|
||||
.borrow_mut()
|
||||
.with_base_mut(|base| callback(init, base)),
|
||||
LayoutBox::TableLevelBox(table_level_box) => {
|
||||
table_level_box.with_base_mut(|base| callback(init, base))
|
||||
},
|
||||
LayoutBox::TaffyItemBox(taffy_item_box) => taffy_item_box
|
||||
.borrow_mut()
|
||||
.with_base_mut(|base| callback(init, base)),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -202,7 +202,7 @@ impl FlexLevelBox {
|
|||
}
|
||||
}
|
||||
|
||||
pub(crate) fn with_base_mut<T>(&mut self, callback: impl Fn(&mut LayoutBoxBase) -> T) -> T {
|
||||
pub(crate) fn with_base_mut<T>(&mut self, callback: impl FnOnce(&mut LayoutBoxBase) -> T) -> T {
|
||||
match self {
|
||||
FlexLevelBox::FlexItem(flex_item_box) => {
|
||||
callback(&mut flex_item_box.independent_formatting_context.base)
|
||||
|
|
|
|||
|
|
@ -290,11 +290,9 @@ impl InlineItem {
|
|||
}
|
||||
}
|
||||
|
||||
pub(crate) fn with_base_mut(&mut self, callback: impl Fn(&mut LayoutBoxBase)) {
|
||||
pub(crate) fn with_base_mut<T>(&mut self, callback: impl FnOnce(&mut LayoutBoxBase) -> T) -> T {
|
||||
match self {
|
||||
InlineItem::StartInlineBox(inline_box) => {
|
||||
callback(&mut inline_box.borrow_mut().base);
|
||||
},
|
||||
InlineItem::StartInlineBox(inline_box) => callback(&mut inline_box.borrow_mut().base),
|
||||
InlineItem::EndInlineBox | InlineItem::TextRun(..) => {
|
||||
unreachable!("Should never have these kind of fragments attached to a DOM node")
|
||||
},
|
||||
|
|
|
|||
|
|
@ -153,7 +153,7 @@ impl BlockLevelBox {
|
|||
}
|
||||
}
|
||||
|
||||
pub(crate) fn with_base_mut<T>(&mut self, callback: impl Fn(&mut LayoutBoxBase) -> T) -> T {
|
||||
pub(crate) fn with_base_mut<T>(&mut self, callback: impl FnOnce(&mut LayoutBoxBase) -> T) -> T {
|
||||
match self {
|
||||
BlockLevelBox::Independent(independent_formatting_context) => {
|
||||
callback(&mut independent_formatting_context.base)
|
||||
|
|
|
|||
|
|
@ -49,56 +49,12 @@ impl BoxTree {
|
|||
#[servo_tracing::instrument(name = "Box Tree Construction", skip_all)]
|
||||
pub(crate) fn construct(context: &LayoutContext, root_element: ServoLayoutNode<'_>) -> Self {
|
||||
let root_element = root_element.to_threadsafe();
|
||||
|
||||
// From https://www.w3.org/TR/css-overflow-3/#overflow-propagation:
|
||||
// > UAs must apply the overflow-* values set on the root element to the viewport when the
|
||||
// > root element’s display value is not none. However, when the root element is an [HTML]
|
||||
// > html element (including XML syntax for HTML) whose overflow value is visible (in both
|
||||
// > axes), and that element has as a child a body element whose display value is also not
|
||||
// > none, user agents must instead apply the overflow-* values of the first such child
|
||||
// > element to the viewport. The element from which the value is propagated must then have a
|
||||
// > used overflow value of visible.
|
||||
let root_style = root_element.style(&context.style_context);
|
||||
|
||||
let mut viewport_overflow = AxesOverflow::from(&*root_style);
|
||||
let mut element_propagating_overflow = root_element;
|
||||
if viewport_overflow.x == Overflow::Visible &&
|
||||
viewport_overflow.y == Overflow::Visible &&
|
||||
!root_style.get_box().display.is_none()
|
||||
{
|
||||
for child in root_element.children() {
|
||||
if !child
|
||||
.as_element()
|
||||
.is_some_and(|element| element.is_body_element_of_html_element_root())
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
let style = child.style(&context.style_context);
|
||||
if !style.get_box().display.is_none() {
|
||||
viewport_overflow = AxesOverflow::from(&*style);
|
||||
element_propagating_overflow = child;
|
||||
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let boxes = construct_for_root_element(context, root_element);
|
||||
|
||||
// Zero box for `:root { display: none }`, one for the root element otherwise.
|
||||
assert!(boxes.len() <= 1);
|
||||
|
||||
if let Some(layout_data) = element_propagating_overflow.inner_layout_data() {
|
||||
if let Some(ref mut layout_box) = *layout_data.self_box.borrow_mut() {
|
||||
layout_box.with_base_mut(|base| {
|
||||
base.base_fragment_info
|
||||
.flags
|
||||
.insert(FragmentFlags::PROPAGATED_OVERFLOW_TO_VIEWPORT)
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
let viewport_overflow = Self::viewport_overflow(root_element, boxes.first());
|
||||
let contents = BlockContainer::BlockLevelBoxes(boxes);
|
||||
let contains_floats = contents.contains_floats();
|
||||
Self {
|
||||
|
|
@ -116,6 +72,63 @@ impl BoxTree {
|
|||
}
|
||||
}
|
||||
|
||||
fn viewport_overflow(
|
||||
root_element: ServoThreadSafeLayoutNode<'_>,
|
||||
root_box: Option<&ArcRefCell<BlockLevelBox>>,
|
||||
) -> AxesOverflow {
|
||||
// From https://www.w3.org/TR/css-overflow-3/#overflow-propagation:
|
||||
// > UAs must apply the overflow-* values set on the root element to the viewport when the
|
||||
// > root element’s display value is not none. However, when the root element is an [HTML]
|
||||
// > html element (including XML syntax for HTML) whose overflow value is visible (in both
|
||||
// > axes), and that element has as a child a body element whose display value is also not
|
||||
// > none, user agents must instead apply the overflow-* values of the first such child
|
||||
// > element to the viewport. The element from which the value is propagated must then have a
|
||||
// > used overflow value of visible.
|
||||
|
||||
// If there is no root box, the root element has `display: none`, so don't propagate.
|
||||
// The spec isn't very clear about what value to use, but the initial value seems fine.
|
||||
// See https://github.com/w3c/csswg-drafts/issues/12649
|
||||
let Some(root_box) = root_box else {
|
||||
return AxesOverflow::default();
|
||||
};
|
||||
|
||||
let propagate_from_body = || {
|
||||
// Unlike what the spec implies, we stop iterating when we find the first <body>,
|
||||
// even if it's not suitable because it lacks a box. This matches other browsers.
|
||||
// See https://github.com/w3c/csswg-drafts/issues/12644
|
||||
let body = root_element.children().find(|child| {
|
||||
child
|
||||
.as_element()
|
||||
.is_some_and(|element| element.is_body_element_of_html_element_root())
|
||||
})?;
|
||||
|
||||
// We only propagate from the <body> if it generates a box. The spec only checks for
|
||||
// `display: none`, but other browsers don't propagate for `display: contents` either.
|
||||
// See https://github.com/w3c/csswg-drafts/issues/12643
|
||||
let body_layout_data = body.inner_layout_data()?;
|
||||
let mut body_box = body_layout_data.self_box.borrow_mut();
|
||||
body_box.as_mut()?.with_base_mut_fold(None, |accum, base| {
|
||||
base.base_fragment_info
|
||||
.flags
|
||||
.insert(FragmentFlags::PROPAGATED_OVERFLOW_TO_VIEWPORT);
|
||||
accum.or_else(|| Some(AxesOverflow::from(&*base.style)))
|
||||
})
|
||||
};
|
||||
|
||||
root_box.borrow_mut().with_base_mut(|base| {
|
||||
let root_overflow = AxesOverflow::from(&*base.style);
|
||||
if root_overflow.x == Overflow::Visible && root_overflow.y == Overflow::Visible {
|
||||
if let Some(body_overflow) = propagate_from_body() {
|
||||
return body_overflow;
|
||||
}
|
||||
}
|
||||
base.base_fragment_info
|
||||
.flags
|
||||
.insert(FragmentFlags::PROPAGATED_OVERFLOW_TO_VIEWPORT);
|
||||
root_overflow
|
||||
})
|
||||
}
|
||||
|
||||
/// This method attempts to incrementally update the box tree from an
|
||||
/// arbitrary node that is not necessarily the document's root element.
|
||||
///
|
||||
|
|
|
|||
|
|
@ -421,7 +421,7 @@ impl TableLevelBox {
|
|||
}
|
||||
}
|
||||
|
||||
pub(crate) fn with_base_mut<T>(&mut self, callback: impl Fn(&mut LayoutBoxBase) -> T) -> T {
|
||||
pub(crate) fn with_base_mut<T>(&mut self, callback: impl FnOnce(&mut LayoutBoxBase) -> T) -> T {
|
||||
match self {
|
||||
TableLevelBox::Caption(caption) => callback(&mut caption.borrow_mut().context.base),
|
||||
TableLevelBox::Cell(cell) => callback(&mut cell.borrow_mut().base),
|
||||
|
|
|
|||
|
|
@ -158,7 +158,7 @@ impl TaffyItemBox {
|
|||
}
|
||||
}
|
||||
|
||||
pub(crate) fn with_base_mut<T>(&mut self, callback: impl Fn(&mut LayoutBoxBase) -> T) -> T {
|
||||
pub(crate) fn with_base_mut<T>(&mut self, callback: impl FnOnce(&mut LayoutBoxBase) -> T) -> T {
|
||||
match &mut self.taffy_level_box {
|
||||
TaffyItemBoxInner::InFlowBox(independent_formatting_context) => {
|
||||
callback(&mut independent_formatting_context.base)
|
||||
|
|
|
|||
52
tests/wpt/meta/MANIFEST.json
vendored
52
tests/wpt/meta/MANIFEST.json
vendored
|
|
@ -232182,6 +232182,58 @@
|
|||
{}
|
||||
]
|
||||
],
|
||||
"overflow-body-propagation-013.html": [
|
||||
"3eb163013e302276845c4e196e74c97f8fd9568e",
|
||||
[
|
||||
null,
|
||||
[
|
||||
[
|
||||
"about:blank",
|
||||
"=="
|
||||
]
|
||||
],
|
||||
{}
|
||||
]
|
||||
],
|
||||
"overflow-body-propagation-014.html": [
|
||||
"26a505a7833a504edd81ce2ee097dcb3190a68c9",
|
||||
[
|
||||
null,
|
||||
[
|
||||
[
|
||||
"/css/css-overflow/overflow-body-propagation-012-ref.html",
|
||||
"=="
|
||||
]
|
||||
],
|
||||
{}
|
||||
]
|
||||
],
|
||||
"overflow-body-propagation-015.html": [
|
||||
"a709a1ff442d3a656c04b3da5313b821bc370942",
|
||||
[
|
||||
null,
|
||||
[
|
||||
[
|
||||
"/css/css-overflow/overflow-body-propagation-012-ref.html",
|
||||
"=="
|
||||
]
|
||||
],
|
||||
{}
|
||||
]
|
||||
],
|
||||
"overflow-body-propagation-016.html": [
|
||||
"35ab59044071343e6b3a31b125af59616327cfcf",
|
||||
[
|
||||
null,
|
||||
[
|
||||
[
|
||||
"/css/css-overflow/overflow-body-propagation-012-ref.html",
|
||||
"=="
|
||||
]
|
||||
],
|
||||
{}
|
||||
]
|
||||
],
|
||||
"overflow-canvas.html": [
|
||||
"e9529cb0bc81202c5689a507435ea088028a97fc",
|
||||
[
|
||||
|
|
|
|||
20
tests/wpt/tests/css/css-overflow/overflow-body-propagation-013.html
vendored
Normal file
20
tests/wpt/tests/css/css-overflow/overflow-body-propagation-013.html
vendored
Normal file
|
|
@ -0,0 +1,20 @@
|
|||
<!DOCTYPE html>
|
||||
<meta charset="utf-8">
|
||||
<title>CSS Test: HTML with display:none and BODY with overflow:scroll</title>
|
||||
<link rel="author" title="Oriol Brufau" href="mailto:obrufau@igalia.com">
|
||||
<link rel="help" href="https://drafts.csswg.org/css-overflow-3/#overflow-propagation">
|
||||
<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/12649">
|
||||
<meta name="assert" content="The <body> doesn't propagate overflow to the
|
||||
viewport when the root element doesn't generate a box. Therefore, there
|
||||
shouldn't be scrollbars.">
|
||||
<link rel="match" href="about:blank">
|
||||
<style>
|
||||
:root {
|
||||
display: none;
|
||||
scrollbar-color: red red;
|
||||
}
|
||||
body {
|
||||
overflow: scroll;
|
||||
}
|
||||
</style>
|
||||
<body></body>
|
||||
42
tests/wpt/tests/css/css-overflow/overflow-body-propagation-014.html
vendored
Normal file
42
tests/wpt/tests/css/css-overflow/overflow-body-propagation-014.html
vendored
Normal file
|
|
@ -0,0 +1,42 @@
|
|||
<!DOCTYPE html>
|
||||
<meta charset="utf-8">
|
||||
<title>CSS Test: two BODY elements</title>
|
||||
<link rel="author" title="Oriol Brufau" href="mailto:obrufau@igalia.com">
|
||||
<link rel="help" href="https://drafts.csswg.org/css-overflow-3/#overflow-propagation">
|
||||
<meta name="assert" content="If there are multiple <body> elements, the
|
||||
one that propagates `overflow` to the viewport is the first one.
|
||||
|
||||
Therefore, the first <body> should get `overflow: visible` and its
|
||||
overflowing contents should be visible. And the second <body> should
|
||||
be able to keep `overflow: hidden` and hide its overflowing contents.">
|
||||
<link rel="match" href="overflow-body-propagation-012-ref.html">
|
||||
<style>
|
||||
body {
|
||||
overflow: hidden;
|
||||
width: 0px;
|
||||
height: 0px;
|
||||
border: solid red;
|
||||
border-width: 0 400px 200px 0;
|
||||
margin-bottom: 0;
|
||||
}
|
||||
body > div {
|
||||
background: green;
|
||||
width: 400px;
|
||||
height: 200px;
|
||||
}
|
||||
#clone {
|
||||
border-color: green;
|
||||
margin-top: 0;
|
||||
}
|
||||
#clone > div {
|
||||
background: red;
|
||||
}
|
||||
</style>
|
||||
<body>
|
||||
<div></div>
|
||||
</body>
|
||||
<script>
|
||||
let clone = document.body.cloneNode(true);
|
||||
clone.id = "clone";
|
||||
document.documentElement.appendChild(clone);
|
||||
</script>
|
||||
49
tests/wpt/tests/css/css-overflow/overflow-body-propagation-015.html
vendored
Normal file
49
tests/wpt/tests/css/css-overflow/overflow-body-propagation-015.html
vendored
Normal file
|
|
@ -0,0 +1,49 @@
|
|||
<!DOCTYPE html>
|
||||
<meta charset="utf-8">
|
||||
<title>CSS Test: two BODY elements</title>
|
||||
<link rel="author" title="Oriol Brufau" href="mailto:obrufau@igalia.com">
|
||||
<link rel="help" href="https://drafts.csswg.org/css-overflow-3/#overflow-propagation">
|
||||
<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1987284">
|
||||
<meta name="assert" content="If there are multiple <body> elements, the
|
||||
one that propagates `overflow` to the viewport is the first one.
|
||||
|
||||
Here we insert a cloned body before the original one, so the original
|
||||
needs to stop propagating because the clone will do it instead.
|
||||
|
||||
Therefore, the cloned <body> should get `overflow: visible` and its
|
||||
overflowing contents should be visible. And the original <body> should
|
||||
be able to keep `overflow: hidden` and hide its overflowing contents.">
|
||||
<link rel="match" href="overflow-body-propagation-012-ref.html">
|
||||
<style>
|
||||
body {
|
||||
overflow: hidden;
|
||||
width: 0px;
|
||||
height: 0px;
|
||||
border: solid green;
|
||||
border-width: 0 400px 200px 0;
|
||||
margin-bottom: 0;
|
||||
}
|
||||
body:not(#clone) {
|
||||
margin-top: 0;
|
||||
}
|
||||
body > div {
|
||||
background: red;
|
||||
width: 400px;
|
||||
height: 200px;
|
||||
}
|
||||
#clone {
|
||||
border-color: red;
|
||||
margin-top: revert;
|
||||
}
|
||||
#clone > div {
|
||||
background: green;
|
||||
}
|
||||
</style>
|
||||
<body>
|
||||
<div></div>
|
||||
</body>
|
||||
<script>
|
||||
let clone = document.body.cloneNode(true);
|
||||
clone.id = "clone";
|
||||
document.documentElement.prepend(clone);
|
||||
</script>
|
||||
49
tests/wpt/tests/css/css-overflow/overflow-body-propagation-016.html
vendored
Normal file
49
tests/wpt/tests/css/css-overflow/overflow-body-propagation-016.html
vendored
Normal file
|
|
@ -0,0 +1,49 @@
|
|||
<!DOCTYPE html>
|
||||
<meta charset="utf-8">
|
||||
<title>CSS Test: two BODY elements</title>
|
||||
<link rel="author" title="Oriol Brufau" href="mailto:obrufau@igalia.com">
|
||||
<link rel="help" href="https://drafts.csswg.org/css-overflow-3/#overflow-propagation">
|
||||
<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/12644">
|
||||
<meta name="assert" content="If there are multiple <body> elements, the
|
||||
one that propagates `overflow` to the viewport would be the first one.
|
||||
|
||||
However, the first one has `display: none`, and overflow propagation
|
||||
can only happen when the element generates a box. Therefore, the viewport
|
||||
shouldn't get scrollbars from the `overflow: scroll` on the first body.
|
||||
|
||||
The second body doesn't propagate either, so it should be able to keep
|
||||
`overflow: hidden` and hide its overflowing contents.">
|
||||
<link rel="match" href="overflow-body-propagation-012-ref.html">
|
||||
<style>
|
||||
html {
|
||||
scrollbar-color: red red;
|
||||
}
|
||||
body {
|
||||
overflow: scroll;
|
||||
width: 0px;
|
||||
height: 0px;
|
||||
border: solid red;
|
||||
border-width: 0 400px 400px 0;
|
||||
}
|
||||
body > div {
|
||||
overflow: hidden;
|
||||
background: green;
|
||||
width: 400px;
|
||||
height: 400px;
|
||||
}
|
||||
#clone {
|
||||
border-color: green;
|
||||
}
|
||||
#clone > div {
|
||||
background: red;
|
||||
}
|
||||
</style>
|
||||
<body>
|
||||
<div></div>
|
||||
</body>
|
||||
<script>
|
||||
let clone = document.body.cloneNode(true);
|
||||
clone.id = "clone";
|
||||
document.documentElement.appendChild(clone);
|
||||
document.body.style.display = "none";
|
||||
</script>
|
||||
Loading…
Add table
Add a link
Reference in a new issue