[carousel] Support :focus for ::column::scroll-marker

This CL enables focusability for column scroll marker and updates node traversal code to include column pseudo elements. For now, we can't properly test focus on column scroll markers with WPT test, as getComputedStyle doesn't allow to get nested pseudo elements. Bug: 365680822 Change-Id: Icc283ff2e853f052ce1df40b4793e0a483cbd88c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6040171 Commit-Queue: Daniil Sakhapov <sakhapov@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/main@{#1388721}
FairyWorld · Nov 30, 2024 · 1059389 · 1059389
1 parent 1abbdf0
commit 1059389
Show file tree

Hide file tree

Showing 5 changed files with 124 additions and 16 deletions.
diff --git a/third_party/blink/renderer/core/dom/node.cc b/third_party/blink/renderer/core/dom/node.cc
@@ -463,6 +463,20 @@ Node* Node::PseudoAwarePreviousSibling() const {
       }
       [[fallthrough]];
     case kPseudoIdScrollMarker:
+      if (const ColumnPseudoElementsVector* columns =
+              parent->GetColumnPseudoElements();
+          columns && !columns->empty()) {
+        return columns->back();
+      }
+      [[fallthrough]];
+    case kPseudoIdColumn:
+      if (auto* column = DynamicTo<ColumnPseudoElement>(this)) {
+        const ColumnPseudoElementsVector* columns =
+            parent->GetColumnPseudoElements();
+        if (column->Index() > 0) {
+          return columns->at(column->Index() - 1u);
+        }
+      }
       if (Node* previous = parent->GetPseudoElement(kPseudoIdMarker)) {
         return previous;
       }
@@ -520,6 +534,20 @@ Node* Node::PseudoAwareNextSibling() const {
       }
       [[fallthrough]];
     case kPseudoIdMarker:
+      if (const ColumnPseudoElementsVector* columns =
+              parent->GetColumnPseudoElements();
+          columns && !columns->empty()) {
+        return columns->front();
+      }
+      [[fallthrough]];
+    case kPseudoIdColumn:
+      if (auto* column = DynamicTo<ColumnPseudoElement>(this)) {
+        const ColumnPseudoElementsVector* columns =
+            parent->GetColumnPseudoElements();
+        if (column->Index() + 1u < columns->size()) {
+          return columns->at(column->Index() + 1u);
+        }
+      }
       if (Node* next = parent->GetPseudoElement(kPseudoIdScrollMarker)) {
         return next;
       }
@@ -641,6 +669,13 @@ Node* Node::PseudoAwareFirstChild() const {
     }
     if (Node* first = current_element->GetPseudoElement(kPseudoIdMarker))
       return first;
+    if (const ColumnPseudoElementsVector* columns =
+            current_element->GetColumnPseudoElements();
+        columns && !columns->empty()) {
+      if (Node* first = columns->front()) {
+        return first;
+      }
+    }
     if (Node* first =
             current_element->GetPseudoElement(kPseudoIdScrollMarker)) {
       return first;
@@ -752,6 +787,13 @@ Node* Node::PseudoAwareLastChild() const {
     if (Node* last = current_element->GetPseudoElement(kPseudoIdScrollMarker)) {
       return last;
     }
+    if (const ColumnPseudoElementsVector* columns =
+            current_element->GetColumnPseudoElements();
+        columns && !columns->empty()) {
+      if (Node* last = columns->back()) {
+        return last;
+      }
+    }
     if (Node* last = current_element->GetPseudoElement(kPseudoIdMarker)) {
       return last;
     }

diff --git a/third_party/blink/renderer/core/dom/scroll_marker_pseudo_element.cc b/third_party/blink/renderer/core/dom/scroll_marker_pseudo_element.cc
@@ -16,17 +16,6 @@
 
 namespace blink {
 
-FocusableState ScrollMarkerPseudoElement::SupportsFocus(
-    UpdateBehavior behavior) const {
-  if (parentNode()->IsColumnPseudoElement()) {
-    // TODO(crbug.com/365680822): This is a ::column::scroll-marker, which
-    // doesn't support :focus. Attempting to focus it would mark for style
-    // recalc, but nobody comes around and recalcs it...
-    return FocusableState::kNotFocusable;
-  }
-  return PseudoElement::SupportsFocus(behavior);
-}
-
 void ScrollMarkerPseudoElement::DefaultEventHandler(Event& event) {
   bool is_click =
       event.IsMouseEvent() && event.type() == event_type_names::kClick;
@@ -52,14 +41,12 @@ void ScrollMarkerPseudoElement::DefaultEventHandler(Event& event) {
       } else if (is_left_or_up_arrow_key) {
         scroll_marker_group_->ActivatePrevScrollMarker(/*focus=*/true);
       } else if (is_click || is_enter_or_space) {
-        ScrollMarkerPseudoElement* scroll_marker = this;
-        scroll_marker_group_->SetSelected(*scroll_marker);
         // parentElement is ::column for column scroll marker and
         // ultimate originating element for regular scroll marker.
         mojom::blink::ScrollIntoViewParamsPtr params =
             scroll_into_view_util::CreateScrollIntoViewParams(
-                *scroll_marker->parentElement()->GetComputedStyle());
-        scroll_marker->ScrollIntoViewNoVisualUpdate(std::move(params));
+                *parentElement()->GetComputedStyle());
+        ScrollIntoViewNoVisualUpdate(std::move(params));
         scroll_marker_group_->SetSelected(*this);
       }
     }

diff --git a/third_party/blink/renderer/core/dom/scroll_marker_pseudo_element.h b/third_party/blink/renderer/core/dom/scroll_marker_pseudo_element.h
@@ -23,7 +23,6 @@ class ScrollMarkerPseudoElement : public PseudoElement {
   void SetSelected(bool value);
   bool IsSelected() const { return is_selected_; }
   int DefaultTabIndex() const override { return 0; }
-  FocusableState SupportsFocus(UpdateBehavior) const final;
   void DefaultEventHandler(Event&) override;
   bool HasActivationBehavior() const final { return true; }
   bool WillRespondToMouseClickEvents() override { return true; }

diff --git a/third_party/blink/web_tests/TestExpectations b/third_party/blink/web_tests/TestExpectations
@@ -5707,6 +5707,9 @@ crbug.com/340389272 virtual/old-text-size-adjust-with-os-font-scale/wpt_internal
 crbug.com/340389272 virtual/old-text-size-adjust/wpt_internal/css/css-size-adjust/text-size-adjust-affects-font-size-large.html [ Failure ]
 crbug.com/340389272 virtual/old-text-size-adjust-with-os-font-scale/wpt_internal/css/css-size-adjust/text-size-adjust-affects-font-size-large.html [ Failure ]
 
+# Blocked on https://github.com/w3c/csswg-drafts/issues/4456 resolution.
+crbug.com/373478544 wpt_internal/css/css-overflow/column-scroll-marker-009.html [ Failure ]
+
 # These tests fail because the old text size adjust wasn't applied if the page didn't run the autosizer.
 crbug.com/340389272 virtual/old-text-size-adjust/wpt_internal/css/css-size-adjust/text-size-adjust-affects-font-size-nested-change.html [ Failure ]
 crbug.com/340389272 virtual/old-text-size-adjust/wpt_internal/css/css-size-adjust/text-size-adjust-affects-font-size-nested.html [ Failure ]

diff --git a/third_party/blink/web_tests/wpt_internal/css/css-overflow/column-scroll-marker-009.html b/third_party/blink/web_tests/wpt_internal/css/css-overflow/column-scroll-marker-009.html
@@ -0,0 +1,77 @@
+<!DOCTYPE html>
+<title>CSS Overflow Test: ::column::scroll-marker supports :focus</title>
+<link rel="help" href="https://github.com/flackr/carousel/tree/main/fragmentation">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-actions.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<style>
+  #container {
+    overflow: hidden;
+    columns: 3;
+    column-fill: auto;
+    gap: 0;
+    orphans: 1;
+    widows: 1;
+    height: 100px;
+    border: 15px solid;
+    line-height: 20px;
+    scroll-marker-group: before;
+    background: yellow;
+  }
+  #container::scroll-marker-group {
+    display: flex;
+    justify-content: space-between;
+    height: 50px;
+    background: cyan;
+  }
+  #container::column::scroll-marker {
+    display: flex;
+    justify-content: center;
+    align-items: center;
+    width: 50px;
+    height: 50px;
+    background: hotpink;
+    content: "*";
+  }
+  #container::column::scroll-marker:focus {
+    background: blue;
+  }
+</style>
+<div style="width:450px;">
+  <div id="container">
+    First column<br>
+    <br>
+    <br>
+    <br>
+    <br>
+    Second column<br>
+    <br>
+    <br>
+    <br>
+    <br>
+    Third column<br>
+    <br>
+    <br>
+    <br>
+    <br>
+    Fourth column<br>
+    <br>
+    <br>
+    <br>
+    <br>
+    Fifth column<br>
+  </div>
+</div>
+<script>
+  promise_test(async t => {
+    actions_promise = new test_driver.Actions()
+    .pointerMove(15, 15)
+    .pointerDown()
+    .pointerUp()
+    .send();
+    await actions_promise;
+    assert_equals(getComputedStyle(container, "::column::scroll-marker").backgroundColor, "blue");
+  });
+</script>