layout: Take into account display: table etc in offset* queries (#32448)

* layout: Take into account `display: table` etc in offset* queries

The specification says that for deciding whether an element should be
used for offset* queries, a browser should take into account whether the
element is a table cell or table. This change makes that happen.

Co-authored-by: Oriol Brufau <obrufau@igalia.com>

* Only tag HTML elements if they are in the HTML namespace

---------

Co-authored-by: Oriol Brufau <obrufau@igalia.com>

components/layout_2020/dom_traversal.rs

@@ -89,14 +89,21 @@ where
         });
 
         let threadsafe_node = node.to_threadsafe();
-        let flags = match threadsafe_node.as_element() {
+        let mut flags = FragmentFlags::empty();
-            Some(element) if element.is_body_element_of_html_element_root() => {
+
-                FragmentFlags::IS_BODY_ELEMENT_OF_HTML_ELEMENT_ROOT
+        if let Some(element) = threadsafe_node.as_html_element() {
+            if element.is_body_element_of_html_element_root() {
+                flags.insert(FragmentFlags::IS_BODY_ELEMENT_OF_HTML_ELEMENT_ROOT);
+            }
+            match element.get_local_name() {
+                &local_name!("br") => {
+                    flags.insert(FragmentFlags::IS_BR_ELEMENT);
                 },
-            Some(element) if element.get_local_name() == &local_name!("br") => {
+                &local_name!("table") | &local_name!("th") | &local_name!("td") => {
-                FragmentFlags::IS_BR_ELEMENT
+                    flags.insert(FragmentFlags::IS_TABLE_TH_OR_TD_ELEMENT);
                 },
-            _ => FragmentFlags::empty(),
+                _ => {},
+            }
         };
 
         Self {

components/layout_2020/fragment_tree/base_fragment.rs

@@ -88,15 +88,19 @@ bitflags! {
         const IS_BODY_ELEMENT_OF_HTML_ELEMENT_ROOT = 0b00000001;
         /// Whether or not the node that created this Fragment is a `<br>` element.
         const IS_BR_ELEMENT = 0b00000010;
+        /// Whether or not the node that created was a `<table>`, `<th>` or
+        /// `<td>` element. Note that this does *not* include elements with
+        /// `display: table` or `display: table-cell`.
+        const IS_TABLE_TH_OR_TD_ELEMENT = 0b00000100;
         /// Whether or not this Fragment was created to contain a replaced element or is
         /// a replaced element.
-        const IS_REPLACED = 0b00000100;
+        const IS_REPLACED = 0b00001000;
         /// Whether or not this Fragment was created to contain a list item marker
         /// with a used value of `list-style-position: outside`.
-        const IS_OUTSIDE_LIST_ITEM_MARKER = 0b00001000;
+        const IS_OUTSIDE_LIST_ITEM_MARKER = 0b00010000;
         /// Avoid painting the fragment, this is used for empty table cells when 'empty-cells' is 'hide'.
         /// This flag doesn't avoid hit-testing.
-        const DO_NOT_PAINT = 0b00010000;
+        const DO_NOT_PAINT = 0b00100000;
     }
 }
 

components/layout_2020/query.rs

@@ -31,7 +31,7 @@ use style::traversal::resolve_style;
 use style::values::generics::font::LineHeight;
 use style_traits::{ParsingMode, ToCss};
 
-use crate::fragment_tree::{Fragment, FragmentFlags, FragmentTree, Tag};
+use crate::fragment_tree::{BoxFragment, Fragment, FragmentFlags, FragmentTree, Tag};
 
 pub fn process_content_box_request(
     requested_node: OpaqueNode,
@@ -428,24 +428,7 @@ fn process_offset_parent_query_inner(
             // Record the paths of the nodes being traversed.
             let parent_node_address = match fragment {
                 Fragment::Box(fragment) | Fragment::Float(fragment) => {
-                    let is_eligible_parent =
+                    let is_eligible_parent = is_eligible_parent(fragment);
-                        match (is_body_element, fragment.style.get_box().position) {
-                            // Spec says the element is eligible as `offsetParent` if any of
-                            // these are true:
-                            //  1) Is the body element
-                            //  2) Is static position *and* is a table or table cell
-                            //  3) Is not static position
-                            // TODO: Handle case 2
-                            (true, _) |
-                            (false, Position::Absolute) |
-                            (false, Position::Fixed) |
-                            (false, Position::Relative) |
-                            (false, Position::Sticky) => true,
-
-                            // Otherwise, it's not a valid parent
-                            (false, Position::Static) => false,
-                        };
-
                     match base.tag {
                         Some(tag) if is_eligible_parent && !tag.is_pseudo() => Some(tag.node),
                         _ => None,
@@ -531,6 +514,29 @@ fn process_offset_parent_query_inner(
     })
 }
 
+/// Returns whether or not the element with the given style and body element determination
+/// is eligible to be a parent element for offset* queries.
+///
+/// From <https://www.w3.org/TR/cssom-view-1/#dom-htmlelement-offsetparent>:
+/// >
+/// > Return the nearest ancestor element of the element for which at least one of the following is
+/// > true and terminate this algorithm if such an ancestor is found:
+/// >   1. The computed value of the position property is not static.
+/// >   2. It is the HTML body element.
+/// >   3. The computed value of the position property of the element is static and the ancestor is
+/// >      one of the following HTML elements: td, th, or table.
+fn is_eligible_parent(fragment: &BoxFragment) -> bool {
+    fragment
+        .base
+        .flags
+        .contains(FragmentFlags::IS_BODY_ELEMENT_OF_HTML_ELEMENT_ROOT) ||
+        fragment.style.get_box().position != Position::Static ||
+        fragment
+            .base
+            .flags
+            .contains(FragmentFlags::IS_TABLE_TH_OR_TD_ELEMENT)
+}
+
 // https://html.spec.whatwg.org/multipage/#the-innertext-idl-attribute
 pub fn process_element_inner_text_query<'dom>(_node: impl LayoutNode<'dom>) -> String {
     "".to_owned()

components/script/layout_dom/element.rs

@@ -69,6 +69,10 @@ impl<'dom> ServoLayoutElement<'dom> {
         ServoLayoutElement { element: el }
     }
 
+    pub(super) fn is_html_element(&self) -> bool {
+        self.element.is_html_element()
+    }
+
     #[inline]
     fn get_attr_enum(&self, namespace: &Namespace, name: &LocalName) -> Option<&AttrValue> {
         self.element.get_attr_for_layout(namespace, name)
@@ -139,7 +143,7 @@ impl<'dom> style::dom::TElement for ServoLayoutElement<'dom> {
     }
 
     fn is_html_element(&self) -> bool {
-        self.element.is_html_element()
+        ServoLayoutElement::is_html_element(self)
     }
 
     fn is_mathml_element(&self) -> bool {

components/script/layout_dom/node.rs

@@ -313,6 +313,11 @@ impl<'dom> ThreadSafeLayoutNode<'dom> for ServoThreadSafeLayoutNode<'dom> {
             })
     }
 
+    fn as_html_element(&self) -> Option<ServoThreadSafeLayoutElement<'dom>> {
+        self.as_element()
+            .filter(|element| element.element.is_html_element())
+    }
+
     fn style_data(&self) -> Option<&'dom StyleData> {
         self.node.style_data()
     }

components/shared/script_layout/wrapper_traits.rs

@@ -230,6 +230,9 @@ pub trait ThreadSafeLayoutNode<'dom>: Clone + Copy + Debug + NodeInfo + PartialE
     /// Returns a ThreadSafeLayoutElement if this is an element, None otherwise.
     fn as_element(&self) -> Option<Self::ConcreteThreadSafeLayoutElement>;
 
+    /// Returns a ThreadSafeLayoutElement if this is an element in an HTML namespace, None otherwise.
+    fn as_html_element(&self) -> Option<Self::ConcreteThreadSafeLayoutElement>;
+
     #[inline]
     fn get_pseudo_element_type(&self) -> PseudoElementType {
         self.as_element()

tests/wpt/meta/css/css-align/blocks/align-content-table-cell.html.ini

@@ -2,20 +2,11 @@
   [vertical-align:top and align-content:start are equivalent]
     expected: FAIL
 
-  [vertical-align:middle and `align-content:unsafe center` are equivalent]
-    expected: FAIL
-
   [vertical-align:bottom and `align-content:unsafe end` are equivalent]
     expected: FAIL
 
   [vertical-align:baseline and align-content:baseline are equivalent]
     expected: FAIL
 
-  [vertical-align:middle and `align-content:safe center` are equivalent if the container is tall]
-    expected: FAIL
-
   [vertical-align:bottom and `align-content:safe end` are equivalent if the container is tall]
     expected: FAIL
-
-  [`align-content:safe center` is equivalent to `unsafe center` even if the specified `height` is short]
-    expected: FAIL

tests/wpt/meta/css/css-tables/tentative/baseline-td.html.ini

@@ -1,6 +1,3 @@
 [baseline-td.html]
-  [table, .display-table 1]
-    expected: FAIL
-
   [table, .display-table 3]
     expected: FAIL

tests/wpt/meta/css/css-tables/tentative/element-sizing.html.ini

@@ -1,3 +0,0 @@
-[element-sizing.html]
-  [table 2]
-    expected: FAIL

tests/wpt/meta/css/css-tables/tentative/td-box-sizing-003.html.ini

@@ -1,7 +1,4 @@
 [td-box-sizing-003.html]
-  [table 8]
-    expected: FAIL
-
   [table 9]
     expected: FAIL
 

tests/wpt/meta/html/rendering/widgets/baseline-alignment-and-overflow.tentative.html.ini

@@ -215,42 +215,6 @@
   [<input type="number" value="0" style="overflow: scroll; appearance: none;">]
     expected: FAIL
 
-  [<input type="range" style="overflow: visible; appearance: auto;">]
-    expected: FAIL
-
-  [<input type="range" style="overflow: hidden; appearance: auto;">]
-    expected: FAIL
-
-  [<input type="range" style="overflow: scroll; appearance: auto;">]
-    expected: FAIL
-
-  [<input type="range" style="overflow: visible; appearance: none;">]
-    expected: FAIL
-
-  [<input type="range" style="overflow: hidden; appearance: none;">]
-    expected: FAIL
-
-  [<input type="range" style="overflow: scroll; appearance: none;">]
-    expected: FAIL
-
-  [<input type="color" value="#000000" style="overflow: visible; appearance: auto;">]
-    expected: FAIL
-
-  [<input type="color" value="#000000" style="overflow: hidden; appearance: auto;">]
-    expected: FAIL
-
-  [<input type="color" value="#000000" style="overflow: scroll; appearance: auto;">]
-    expected: FAIL
-
-  [<input type="color" value="#000000" style="overflow: visible; appearance: none;">]
-    expected: FAIL
-
-  [<input type="color" value="#000000" style="overflow: hidden; appearance: none;">]
-    expected: FAIL
-
-  [<input type="color" value="#000000" style="overflow: scroll; appearance: none;">]
-    expected: FAIL
-
   [<input type="checkbox" style="overflow: visible; appearance: auto;">]
     expected: FAIL
 
@@ -263,12 +227,6 @@
   [<input type="checkbox" style="overflow: visible; appearance: none;">]
     expected: FAIL
 
-  [<input type="checkbox" style="overflow: hidden; appearance: none;">]
-    expected: FAIL
-
-  [<input type="checkbox" style="overflow: scroll; appearance: none;">]
-    expected: FAIL
-
   [<input type="radio" style="overflow: visible; appearance: auto;">]
     expected: FAIL
 
@@ -281,12 +239,6 @@
   [<input type="radio" style="overflow: visible; appearance: none;">]
     expected: FAIL
 
-  [<input type="radio" style="overflow: hidden; appearance: none;">]
-    expected: FAIL
-
-  [<input type="radio" style="overflow: scroll; appearance: none;">]
-    expected: FAIL
-
   [<input type="file" style="overflow: visible; appearance: auto;">]
     expected: FAIL
 
@@ -326,39 +278,9 @@
   [<input type="image" src="data:,broken" alt="x" style="overflow: visible; appearance: auto;">]
     expected: FAIL
 
-  [<input type="image" src="data:,broken" alt="x" style="overflow: hidden; appearance: auto;">]
-    expected: FAIL
-
-  [<input type="image" src="data:,broken" alt="x" style="overflow: scroll; appearance: auto;">]
-    expected: FAIL
-
   [<input type="image" src="data:,broken" alt="x" style="overflow: visible; appearance: none;">]
     expected: FAIL
 
-  [<input type="image" src="data:,broken" alt="x" style="overflow: hidden; appearance: none;">]
-    expected: FAIL
-
-  [<input type="image" src="data:,broken" alt="x" style="overflow: scroll; appearance: none;">]
-    expected: FAIL
-
-  [<input type="image" src="data:(png)" alt="x" style="overflow: visible; appearance: auto;">]
-    expected: FAIL
-
-  [<input type="image" src="data:(png)" alt="x" style="overflow: hidden; appearance: auto;">]
-    expected: FAIL
-
-  [<input type="image" src="data:(png)" alt="x" style="overflow: scroll; appearance: auto;">]
-    expected: FAIL
-
-  [<input type="image" src="data:(png)" alt="x" style="overflow: visible; appearance: none;">]
-    expected: FAIL
-
-  [<input type="image" src="data:(png)" alt="x" style="overflow: hidden; appearance: none;">]
-    expected: FAIL
-
-  [<input type="image" src="data:(png)" alt="x" style="overflow: scroll; appearance: none;">]
-    expected: FAIL
-
   [<input type="reset" value="x" style="overflow: visible; appearance: auto;">]
     expected: FAIL