Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-06-06 16:45:39 +00:00
Code Issues Projects Releases Packages Wiki Activity

layout: Do not inherit node and fragment flags in anonymous boxes (#31586)

Browse source 
This doesn't really have observable behavior right now, as much as I
tried to trigger some kind of bug. On the other hand, it's just wrong
and is very obvious when you dump the Fragment tree. If you create a
`display: table-cell` that is a child of the `<body>` all parts of the
anonymous table are flagged as if they are the `<body>` element.
This commit is contained in:
Martin Robinson 2024-03-09 10:13:19 +01:00 • committed by GitHub
parent 55f908653f
commit 1f23ec2b27
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 73 additions and 38 deletions
Download patch file Download diff file Expand all files Collapse all files

33
components/layout_2020/dom_traversal.rs
View file

@ -5,6 +5,7 @@
use std::borrow::Cow; use std::borrow::Cow;
use html5ever::{local_name, LocalName}; use html5ever::{local_name, LocalName};
use log::warn;
use script_layout_interface::wrapper_traits::{ThreadSafeLayoutElement, ThreadSafeLayoutNode}; use script_layout_interface::wrapper_traits::{ThreadSafeLayoutElement, ThreadSafeLayoutNode};
use servo_arc::Arc as ServoArc; use servo_arc::Arc as ServoArc;
use style::properties::ComputedValues; use style::properties::ComputedValues;
@ -27,7 +28,7 @@ pub(crate) enum WhichPseudoElement {
/// avoid having to repeat the same arguments in argument lists. /// avoid having to repeat the same arguments in argument lists.
#[derive(Clone)] #[derive(Clone)]
pub(crate) struct NodeAndStyleInfo<Node> { pub(crate) struct NodeAndStyleInfo<Node> {
pub node: Node, pub node: Option<Node>,
pub pseudo_element_type: Option<WhichPseudoElement>, pub pseudo_element_type: Option<WhichPseudoElement>,
pub style: ServoArc<ComputedValues>, pub style: ServoArc<ComputedValues>,
} }
@ -39,7 +40,7 @@ impl<Node> NodeAndStyleInfo<Node> {
style: ServoArc<ComputedValues>, style: ServoArc<ComputedValues>,
) -> Self { ) -> Self {
Self { Self {
node, node: Some(node),
pseudo_element_type: Some(pseudo_element_type), pseudo_element_type: Some(pseudo_element_type),
style, style,
} }
@ -47,7 +48,7 @@ impl<Node> NodeAndStyleInfo<Node> {
pub(crate) fn new(node: Node, style: ServoArc<ComputedValues>) -> Self { pub(crate) fn new(node: Node, style: ServoArc<ComputedValues>) -> Self {
Self { Self {
node, node: Some(node),
pseudo_element_type: None, pseudo_element_type: None,
style, style,
} }
@ -55,6 +56,14 @@ impl<Node> NodeAndStyleInfo<Node> {
} }
impl<Node: Clone> NodeAndStyleInfo<Node> { impl<Node: Clone> NodeAndStyleInfo<Node> {
pub(crate) fn new_anonymous(&self, style: ServoArc<ComputedValues>) -> Self {
Self {
node: None,
pseudo_element_type: self.pseudo_element_type,
style,
}
}
pub(crate) fn new_replacing_style(&self, style: ServoArc<ComputedValues>) -> Self { pub(crate) fn new_replacing_style(&self, style: ServoArc<ComputedValues>) -> Self {
Self { Self {
node: self.node.clone(), node: self.node.clone(),
@ -69,12 +78,17 @@ where
Node: NodeExt<'dom>, Node: NodeExt<'dom>,
{ {
fn from(info: &NodeAndStyleInfo<Node>) -> Self { fn from(info: &NodeAndStyleInfo<Node>) -> Self {
let node = match info.node {
Some(node) => node,
None => return Self::anonymous(),
};
let pseudo = info.pseudo_element_type.map(|pseudo| match pseudo { let pseudo = info.pseudo_element_type.map(|pseudo| match pseudo {
WhichPseudoElement::Before => PseudoElement::Before, WhichPseudoElement::Before => PseudoElement::Before,
WhichPseudoElement::After => PseudoElement::After, WhichPseudoElement::After => PseudoElement::After,
}); });
let threadsafe_node = info.node.to_threadsafe(); let threadsafe_node = node.to_threadsafe();
let flags = match threadsafe_node.as_element() { let flags = match threadsafe_node.as_element() {
Some(element) if element.is_body_element_of_html_element_root() => { Some(element) if element.is_body_element_of_html_element_root() => {
FragmentFlags::IS_BODY_ELEMENT_OF_HTML_ELEMENT_ROOT FragmentFlags::IS_BODY_ELEMENT_OF_HTML_ELEMENT_ROOT
@ -86,7 +100,7 @@ where
}; };
Self { Self {
tag: Tag::new_pseudo(threadsafe_node.opaque(), pseudo), tag: Some(Tag::new_pseudo(threadsafe_node.opaque(), pseudo)),
flags, flags,
} }
} }
@ -302,8 +316,15 @@ impl NonReplacedContents {
) where ) where
Node: NodeExt<'dom>, Node: NodeExt<'dom>,
{ {
let node = match info.node {
Some(node) => node,
None => {
warn!("Tried to traverse an anonymous node!");
return;
},
};
match self { match self {
NonReplacedContents::OfElement => traverse_children_of(info.node, context, handler), NonReplacedContents::OfElement => traverse_children_of(node, context, handler),
NonReplacedContents::OfPseudoElement(items) => { NonReplacedContents::OfPseudoElement(items) => {
traverse_pseudo_element_contents(info, context, handler, items) traverse_pseudo_element_contents(info, context, handler, items)
}, },

4
components/layout_2020/flexbox/construct.rs
View file

@ -160,9 +160,7 @@ where
self.context, self.context,
self.text_decoration_line, self.text_decoration_line,
); );
let info = &self let info = &self.info.new_anonymous(anonymous_style.clone().unwrap());
.info
.new_replacing_style(anonymous_style.clone().unwrap());
IndependentFormattingContext::NonReplaced(NonReplacedFormattingContext { IndependentFormattingContext::NonReplaced(NonReplacedFormattingContext {
base_fragment_info: info.into(), base_fragment_info: info.into(),
style: info.style.clone(), style: info.style.clone(),

4
components/layout_2020/flow/construct.rs
View file

@ -317,7 +317,7 @@ where
self.current_inline_level_boxes() self.current_inline_level_boxes()
.push(ArcRefCell::new(InlineLevelBox::Atomic(ifc))); .push(ArcRefCell::new(InlineLevelBox::Atomic(ifc)));
} else { } else {
let anonymous_info = self.info.new_replacing_style(ifc.style().clone()); let anonymous_info = self.info.new_anonymous(ifc.style().clone());
let table_block = ArcRefCell::new(BlockLevelBox::Independent(ifc)); let table_block = ArcRefCell::new(BlockLevelBox::Independent(ifc));
self.block_level_boxes.push(BlockLevelJob { self.block_level_boxes.push(BlockLevelJob {
info: anonymous_info, info: anonymous_info,
@ -690,7 +690,7 @@ where
); );
std::mem::swap(&mut self.ongoing_inline_formatting_context, &mut ifc); std::mem::swap(&mut self.ongoing_inline_formatting_context, &mut ifc);
let info = self.info.new_replacing_style(anonymous_style.clone()); let info = self.info.new_anonymous(anonymous_style.clone());
self.block_level_boxes.push(BlockLevelJob { self.block_level_boxes.push(BlockLevelJob {
info, info,
// FIXME(nox): We should be storing this somewhere. // FIXME(nox): We should be storing this somewhere.

15
components/layout_2020/fragment_tree/base_fragment.rs
View file

@ -47,8 +47,8 @@ impl BaseFragment {
/// Information necessary to construct a new BaseFragment. /// Information necessary to construct a new BaseFragment.
#[derive(Clone, Copy, Debug, Serialize)] #[derive(Clone, Copy, Debug, Serialize)]
pub(crate) struct BaseFragmentInfo { pub(crate) struct BaseFragmentInfo {
/// The tag to use for the new BaseFragment. /// The tag to use for the new BaseFragment, if it is not an anonymous Fragment.
pub tag: Tag, pub tag: Option<Tag>,
/// The flags to use for the new BaseFragment. /// The flags to use for the new BaseFragment.
pub flags: FragmentFlags, pub flags: FragmentFlags,
@ -57,7 +57,14 @@ pub(crate) struct BaseFragmentInfo {
impl BaseFragmentInfo { impl BaseFragmentInfo {
pub(crate) fn new_for_node(node: OpaqueNode) -> Self { pub(crate) fn new_for_node(node: OpaqueNode) -> Self {
Self { Self {
tag: Tag::new(node), tag: Some(Tag::new(node)),
flags: FragmentFlags::empty(),
}
}
pub(crate) fn anonymous() -> Self {
Self {
tag: None,
flags: FragmentFlags::empty(), flags: FragmentFlags::empty(),
} }
} }
@ -66,7 +73,7 @@ impl BaseFragmentInfo {
impl From<BaseFragmentInfo> for BaseFragment { impl From<BaseFragmentInfo> for BaseFragment {
fn from(info: BaseFragmentInfo) -> Self { fn from(info: BaseFragmentInfo) -> Self {
Self { Self {
tag: Some(info.tag), tag: info.tag,
debug_id: DebugId::new(), debug_id: DebugId::new(),
flags: info.flags, flags: info.flags,
} }

10
components/layout_2020/lists.rs
View file

@ -2,6 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this * License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
use log::warn;
use style::properties::longhands::list_style_type::computed_value::T as ListStyleType; use style::properties::longhands::list_style_type::computed_value::T as ListStyleType;
use style::properties::style_structs; use style::properties::style_structs;
use style::values::computed::Image; use style::values::computed::Image;
@ -20,12 +21,19 @@ where
Node: NodeExt<'dom>, Node: NodeExt<'dom>,
{ {
let style = info.style.get_list(); let style = info.style.get_list();
let node = match info.node {
Some(node) => node,
None => {
warn!("Tried to make a marker for an anonymous node!");
return None;
},
};
// https://drafts.csswg.org/css-lists/#marker-image // https://drafts.csswg.org/css-lists/#marker-image
let marker_image = || match &style.list_style_image { let marker_image = || match &style.list_style_image {
Image::Url(url) => Some(vec![ Image::Url(url) => Some(vec![
PseudoElementContentItem::Replaced(ReplacedContent::from_image_url( PseudoElementContentItem::Replaced(ReplacedContent::from_image_url(
info.node, context, url, node, context, url,
)?), )?),
PseudoElementContentItem::Text(" ".into()), PseudoElementContentItem::Text(" ".into()),
]), ]),

43
components/layout_2020/table/construct.rs
View file

@ -26,7 +26,7 @@ use crate::formatting_contexts::{
IndependentFormattingContext, NonReplacedFormattingContext, IndependentFormattingContext, NonReplacedFormattingContext,
NonReplacedFormattingContextContents, NonReplacedFormattingContextContents,
}; };
use crate::fragment_tree::{BaseFragmentInfo, FragmentFlags, Tag}; use crate::fragment_tree::BaseFragmentInfo;
use crate::style_ext::{DisplayGeneratingBox, DisplayLayoutInternal}; use crate::style_ext::{DisplayGeneratingBox, DisplayLayoutInternal};
/// A reference to a slot and its coordinates in the table /// A reference to a slot and its coordinates in the table
@ -87,7 +87,7 @@ impl Table {
&PseudoElement::ServoAnonymousTable, &PseudoElement::ServoAnonymousTable,
&parent_info.style, &parent_info.style,
); );
let anonymous_info = parent_info.new_replacing_style(anonymous_style.clone()); let anonymous_info = parent_info.new_anonymous(anonymous_style.clone());
let mut table_builder = let mut table_builder =
TableBuilderTraversal::new(context, &anonymous_info, propagated_text_decoration_line); TableBuilderTraversal::new(context, &anonymous_info, propagated_text_decoration_line);
@ -642,7 +642,7 @@ where
&PseudoElement::ServoAnonymousTableCell, &PseudoElement::ServoAnonymousTableCell,
&self.info.style, &self.info.style,
); );
let anonymous_info = self.info.new_replacing_style(anonymous_style); let anonymous_info = self.info.new_anonymous(anonymous_style);
let mut row_builder = let mut row_builder =
TableRowBuilder::new(self, &anonymous_info, self.current_text_decoration_line); TableRowBuilder::new(self, &anonymous_info, self.current_text_decoration_line);
@ -756,8 +756,12 @@ where
::std::mem::forget(box_slot) ::std::mem::forget(box_slot)
}, },
DisplayLayoutInternal::TableColumn => { DisplayLayoutInternal::TableColumn => {
let node = info.node.to_threadsafe(); let span = info
let span = (node.get_span().unwrap_or(1) as usize).min(1000); .node
.and_then(|node| node.to_threadsafe().get_span())
.unwrap_or(1)
.min(1000);
for _ in 0..span + 1 { for _ in 0..span + 1 {
self.builder.table.columns.push(TableTrack { self.builder.table.columns.push(TableTrack {
base_fragment_info: info.into(), base_fragment_info: info.into(),
@ -785,8 +789,11 @@ where
let first_column = self.builder.table.columns.len(); let first_column = self.builder.table.columns.len();
if column_group_builder.columns.is_empty() { if column_group_builder.columns.is_empty() {
let node = info.node.to_threadsafe(); let span = info
let span = (node.get_span().unwrap_or(1) as usize).min(1000); .node
.and_then(|node| node.to_threadsafe().get_span())
.unwrap_or(1)
.min(1000) as usize;
self.builder.table.columns.extend( self.builder.table.columns.extend(
repeat(TableTrack { repeat(TableTrack {
@ -894,7 +901,7 @@ where
&PseudoElement::ServoAnonymousTableCell, &PseudoElement::ServoAnonymousTableCell,
&self.info.style, &self.info.style,
); );
let anonymous_info = self.info.new_replacing_style(anonymous_style); let anonymous_info = self.info.new_anonymous(anonymous_style);
let mut builder = let mut builder =
BlockContainerBuilder::new(context, &anonymous_info, self.text_decoration_line); BlockContainerBuilder::new(context, &anonymous_info, self.text_decoration_line);
@ -914,22 +921,13 @@ where
} }
} }
let tag = Tag::new_pseudo(
self.info.node.opaque(),
Some(PseudoElement::ServoAnonymousTableCell),
);
let base_fragment_info = BaseFragmentInfo {
tag,
flags: FragmentFlags::empty(),
};
let block_container = builder.finish(); let block_container = builder.finish();
self.table_traversal.builder.add_cell(TableSlotCell { self.table_traversal.builder.add_cell(TableSlotCell {
contents: BlockFormattingContext::from_block_container(block_container), contents: BlockFormattingContext::from_block_container(block_container),
colspan: 1, colspan: 1,
rowspan: 1, rowspan: 1,
style: anonymous_info.style, style: anonymous_info.style,
base_fragment_info, base_fragment_info: BaseFragmentInfo::anonymous(),
}); });
} }
} }
@ -965,9 +963,12 @@ where
// 65534 and `colspan` to 1000, so we also enforce the same limits // 65534 and `colspan` to 1000, so we also enforce the same limits
// when dealing with arbitrary DOM elements (perhaps created via // when dealing with arbitrary DOM elements (perhaps created via
// script). // script).
let node = info.node.to_threadsafe(); let (rowspan, colspan) = info.node.map_or((1, 1), |node| {
let rowspan = (node.get_rowspan().unwrap_or(1) as usize).min(65534); let node = node.to_threadsafe();
let colspan = (node.get_colspan().unwrap_or(1) as usize).min(1000); let rowspan = node.get_rowspan().unwrap_or(1).min(65534) as usize;
let colspan = node.get_colspan().unwrap_or(1).min(1000) as usize;
(rowspan, colspan)
});
let contents = match contents.try_into() { let contents = match contents.try_into() {
Ok(non_replaced_contents) => { Ok(non_replaced_contents) => {

2
components/layout_2020/table/mod.rs
View file

@ -198,7 +198,7 @@ impl TableSlotCell {
/// Get the node id of this cell's [`BaseFragmentInfo`]. This is used for unit tests. /// Get the node id of this cell's [`BaseFragmentInfo`]. This is used for unit tests.
pub fn node_id(&self) -> usize { pub fn node_id(&self) -> usize {
self.base_fragment_info.tag.node.0 self.base_fragment_info.tag.map_or(0, |tag| tag.node.0)
} }
} }