From 136665290e1d6539771ab566cffc9a8f6942d42c Mon Sep 17 00:00:00 2001 From: Daniil Sakhapov Date: Fri, 29 Nov 2024 15:52:48 +0000 Subject: [PATCH] [carousel] Make ::scroll-button() layout box siblings of its scroller Bug: 370067113 Change-Id: Ibf90e8d0d4796df8ba82761956f4ea390800800a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6058307 Reviewed-by: Rune Lillesveen Commit-Queue: Daniil Sakhapov Cr-Commit-Position: refs/heads/main@{#1389818} --- .../blink/renderer/core/dom/element.cc | 12 +- third_party/blink/renderer/core/dom/element.h | 4 - .../core/dom/layout_tree_builder_traversal.cc | 104 ++++++++++++++---- .../scroll-buttons-activation.html | 4 - 4 files changed, 89 insertions(+), 35 deletions(-) diff --git a/third_party/blink/renderer/core/dom/element.cc b/third_party/blink/renderer/core/dom/element.cc index 995ab3f5329c6c..43523e31e17d0b 100644 --- a/third_party/blink/renderer/core/dom/element.cc +++ b/third_party/blink/renderer/core/dom/element.cc @@ -3320,6 +3320,10 @@ void Element::AttachLayoutTree(AttachContext& context) { } AttachPseudoElement(kPseudoIdScrollMarkerGroupBefore, context); + AttachPseudoElement(kPseudoIdScrollUpButton, context); + AttachPseudoElement(kPseudoIdScrollLeftButton, context); + AttachPseudoElement(kPseudoIdScrollRightButton, context); + AttachPseudoElement(kPseudoIdScrollDownButton, context); AttachContext children_context(context); LayoutObject* layout_object = nullptr; @@ -4427,10 +4431,10 @@ void Element::RebuildLayoutTree(WhitespaceAttacher& whitespace_attacher) { RebuildPseudoElementLayoutTree(kPseudoIdCheckMark, *child_attacher); RebuildPseudoElementLayoutTree(kPseudoIdBefore, *child_attacher); RebuildPseudoElementLayoutTree(kPseudoIdMarker, *child_attacher); - RebuildPseudoElementLayoutTree(kPseudoIdScrollRightButton, *child_attacher); - RebuildPseudoElementLayoutTree(kPseudoIdScrollLeftButton, *child_attacher); - RebuildPseudoElementLayoutTree(kPseudoIdScrollDownButton, *child_attacher); - RebuildPseudoElementLayoutTree(kPseudoIdScrollUpButton, *child_attacher); + RebuildPseudoElementLayoutTree(kPseudoIdScrollDownButton, local_attacher); + RebuildPseudoElementLayoutTree(kPseudoIdScrollRightButton, local_attacher); + RebuildPseudoElementLayoutTree(kPseudoIdScrollLeftButton, local_attacher); + RebuildPseudoElementLayoutTree(kPseudoIdScrollUpButton, local_attacher); RebuildPseudoElementLayoutTree(kPseudoIdScrollMarkerGroupBefore, local_attacher); RebuildPseudoElementLayoutTree(kPseudoIdBackdrop, *child_attacher); diff --git a/third_party/blink/renderer/core/dom/element.h b/third_party/blink/renderer/core/dom/element.h index 3afcd8c6680eee..5b66f025b59348 100644 --- a/third_party/blink/renderer/core/dom/element.h +++ b/third_party/blink/renderer/core/dom/element.h @@ -1783,10 +1783,6 @@ class CORE_EXPORT Element : public ContainerNode, public Animatable { void DetachPseudoElement(PseudoId, bool performing_reattach); void AttachPrecedingPseudoElements(AttachContext& context) { - AttachPseudoElement(kPseudoIdScrollUpButton, context); - AttachPseudoElement(kPseudoIdScrollDownButton, context); - AttachPseudoElement(kPseudoIdScrollLeftButton, context); - AttachPseudoElement(kPseudoIdScrollRightButton, context); AttachPseudoElement(kPseudoIdMarker, context); AttachPseudoElement(kPseudoIdCheckMark, context); AttachPseudoElement(kPseudoIdBefore, context); diff --git a/third_party/blink/renderer/core/dom/layout_tree_builder_traversal.cc b/third_party/blink/renderer/core/dom/layout_tree_builder_traversal.cc index 8e5875f73d6952..d6b9df67ec9699 100644 --- a/third_party/blink/renderer/core/dom/layout_tree_builder_traversal.cc +++ b/third_party/blink/renderer/core/dom/layout_tree_builder_traversal.cc @@ -33,6 +33,7 @@ #include "third_party/blink/renderer/core/html_names.h" #include "third_party/blink/renderer/core/layout/layout_object.h" #include "third_party/blink/renderer/core/layout/layout_view.h" +#include "third_party/blink/renderer/core/style/computed_style_constants.h" namespace blink { @@ -45,6 +46,21 @@ static bool IsLayoutObjectReparented(const LayoutObject* layout_object) { return layout_object->IsInTopOrViewTransitionLayer(); } +static Node* PreviousLayoutSiblingOfElement(Element& element) { + Node* originating_prev = LayoutTreeBuilderTraversal::PreviousSibling(element); + Element* originating_prev_element = DynamicTo(originating_prev); + if (originating_prev_element && + originating_prev_element->GetComputedStyle() && + originating_prev_element->GetComputedStyle() + ->HasScrollMarkerGroupAfter()) { + if (Element* pseudo = originating_prev_element->GetPseudoElement( + kPseudoIdScrollMarkerGroupAfter)) { + return pseudo; + } + } + return originating_prev; +} + ContainerNode* LayoutTreeBuilderTraversal::Parent(const Node& node) { // TODO(hayato): Uncomment this once we can be sure // LayoutTreeBuilderTraversal::parent() is used only for a node which is @@ -79,7 +95,7 @@ LayoutObject* LayoutTreeBuilderTraversal::ParentLayoutObject(const Node& node) { // Parent of ::scroll-marker-group should be layout parent of its // originating element. if (node.IsScrollMarkerGroupPseudoElement()) { - search_start_node = To(node).UltimateOriginatingElement(); + search_start_node = node.parentNode(); } ContainerNode* parent = LayoutTreeBuilderTraversal::LayoutParent(*search_start_node); @@ -466,7 +482,8 @@ Node* LayoutTreeBuilderTraversal::Next(const Node& node, } // Checks if current or (next/prev) sibling is either ::scroll-marker-group -// or element with scroll-marker-group property set. +// or element with scroll-marker-group property set, +// or ::scroll-button(), or element with scroll-button. static inline bool AreBoxTreeOrderSiblings(const Node& current, Node* sibling) { if (current.IsScrollMarkerGroupPseudoElement()) { return false; @@ -486,6 +503,26 @@ static inline bool AreBoxTreeOrderSiblings(const Node& current, Node* sibling) { return false; } } + if (current.IsScrollButtonPseudoElement()) { + return false; + } + if (Element* sibling_element = DynamicTo(sibling)) { + if (sibling_element->IsScrollButtonPseudoElement()) { + return false; + } + if (sibling_element->GetPseudoElement(kPseudoIdScrollUpButton)) { + return false; + } + if (sibling_element->GetPseudoElement(kPseudoIdScrollLeftButton)) { + return false; + } + if (sibling_element->GetPseudoElement(kPseudoIdScrollRightButton)) { + return false; + } + if (sibling_element->GetPseudoElement(kPseudoIdScrollDownButton)) { + return false; + } + } return true; } @@ -499,13 +536,14 @@ static inline bool AreBoxTreeOrderSiblings(const Node& current, Node* sibling) { // OE - originating element // PS - previous sibling of OE // NS - next sibling of OE +// SB - scroll buttons // SMGB - ::scroll-marker-group of OE with scroll-marker-group: before // SMGA - ::scroll-marker-group of OE with scroll-marker-group: after // B - ::before of OE // A - ::after of OE // Node tree: -// (PS) (OE) (NS) -// (SMGB) (B) (A) (SMGA) +// (PS) (OE) (NS) +// (SMGB) (SB) (B) (A) (SMGA) // Node tree is input (`node`), return output based on layout tree. static Node* NextLayoutSiblingInBoxTreeOrder(const Node& node) { Node* next = LayoutTreeBuilderTraversal::NextSibling(node); @@ -537,14 +575,28 @@ static Node* NextLayoutSiblingInBoxTreeOrder(const Node& node) { return pseudo; } } - // From SMGB, return OE. + // From SMGB. if (element->IsScrollMarkerGroupBeforePseudoElement()) { - return To(element)->UltimateOriginatingElement(); + // return SB, if found. + if (next && next->IsScrollButtonPseudoElement()) { + return next; + } + // return OE, if not. + return element->parentNode(); + } + // From SB. + if (element->IsScrollButtonPseudoElement()) { + // return next SB, if found. + if (next && next->IsScrollButtonPseudoElement()) { + return next; + } + // return OE, if not. + return element->parentNode(); } // From SMGA, return NS, but check if NS has SMGB, then return NS's SMGB. if (element->IsScrollMarkerGroupAfterPseudoElement()) { - Node* originating_next = LayoutTreeBuilderTraversal::NextSibling( - *To(element)->UltimateOriginatingElement()); + Node* originating_next = + LayoutTreeBuilderTraversal::NextSibling(*element->parentNode()); Element* originating_next_element = DynamicTo(originating_next); if (originating_next_element && originating_next_element->GetComputedStyle() && @@ -619,6 +671,21 @@ static Node* PreviousLayoutSiblingInBoxTreeOrder(const Node& node) { if (!element) { return previous; } + if (element->IsScrollMarkerGroupAfterPseudoElement()) { + return element->parentNode(); + } + if (Element* pseudo = element->GetPseudoElement(kPseudoIdScrollDownButton)) { + return pseudo; + } + if (Element* pseudo = element->GetPseudoElement(kPseudoIdScrollRightButton)) { + return pseudo; + } + if (Element* pseudo = element->GetPseudoElement(kPseudoIdScrollLeftButton)) { + return pseudo; + } + if (Element* pseudo = element->GetPseudoElement(kPseudoIdScrollUpButton)) { + return pseudo; + } if (element->GetComputedStyle() && element->GetComputedStyle()->HasScrollMarkerGroupBefore()) { if (Element* pseudo = @@ -626,23 +693,14 @@ static Node* PreviousLayoutSiblingInBoxTreeOrder(const Node& node) { return pseudo; } } - if (element->IsScrollMarkerGroupAfterPseudoElement()) { - return To(element)->UltimateOriginatingElement(); + if (element->IsScrollButtonPseudoElement()) { + if (previous && previous->IsScrollMarkerGroupBeforePseudoElement()) { + return previous; + } + return PreviousLayoutSiblingOfElement(*element->parentElement()); } if (element->IsScrollMarkerGroupBeforePseudoElement()) { - Node* originating_prev = LayoutTreeBuilderTraversal::PreviousSibling( - *To(element)->UltimateOriginatingElement()); - Element* originating_prev_element = DynamicTo(originating_prev); - if (originating_prev_element && - originating_prev_element->GetComputedStyle() && - originating_prev_element->GetComputedStyle() - ->HasScrollMarkerGroupAfter()) { - if (Element* pseudo = originating_prev_element->GetPseudoElement( - kPseudoIdScrollMarkerGroupAfter)) { - return pseudo; - } - } - return originating_prev; + return PreviousLayoutSiblingOfElement(*element->parentElement()); } return previous; } diff --git a/third_party/blink/web_tests/wpt_internal/css/css-overflow/scroll-buttons-activation.html b/third_party/blink/web_tests/wpt_internal/css/css-overflow/scroll-buttons-activation.html index c4777b10252347..7223dbbdaf7b64 100644 --- a/third_party/blink/web_tests/wpt_internal/css/css-overflow/scroll-buttons-activation.html +++ b/third_party/blink/web_tests/wpt_internal/css/css-overflow/scroll-buttons-activation.html @@ -46,10 +46,6 @@ display: flex; height: 20px; width: 20px; - top: 0px; - left: 0px; - position: absolute; - z-index: 99; } #scroller div::scroll-marker {