Create interest style for spat nav and remove force outline flag

This CL creates a new style selector (or rather refactors the old one)
for interest rings in spat nav. It also removes the
SpatialNavigationForcesOutline flag and always shows the interest ring
when using spat nav. See crrev.com/c/1344252

A future CL will remove the FocuslessSpatialNavigation flag.

Bug: 906640
Change-Id: I8bb0e367a0dfd08a7bd48a3f940e58ac60875b36
Reviewed-on: https://chromium-review.googlesource.com/c/1468590
Auto-Submit: Mehran Mahmoudi <mahmoudi@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632706}

third_party/blink/renderer/core/css/css_selector.cc

@@ -281,6 +281,7 @@ PseudoId CSSSelector::GetPseudoId(PseudoType type) {
     case kPseudoFullScreenAncestor:
     case kPseudoFullscreen:
     case kPseudoSpatialNavigationFocus:
+    case kPseudoSpatialNavigationInterest:
     case kPseudoIsHtml:
     case kPseudoListBox:
     case kPseudoHostHasAppearance:
@@ -314,6 +315,8 @@ const static NameToPseudoStruct kPseudoTypeWithoutArgumentsMap[] = {
      CSSSelector::kPseudoHostHasAppearance},
     {"-internal-spatial-navigation-focus",
      CSSSelector::kPseudoSpatialNavigationFocus},
+    {"-internal-spatial-navigation-interest",
+     CSSSelector::kPseudoSpatialNavigationInterest},
     {"-internal-video-persistent", CSSSelector::kPseudoVideoPersistent},
     {"-internal-video-persistent-ancestor",
      CSSSelector::kPseudoVideoPersistentAncestor},
@@ -565,6 +568,7 @@ void CSSSelector::UpdatePseudoType(const AtomicString& value,
     case kPseudoIsHtml:
     case kPseudoListBox:
     case kPseudoSpatialNavigationFocus:
+    case kPseudoSpatialNavigationInterest:
     case kPseudoVideoPersistent:
     case kPseudoVideoPersistentAncestor:
       if (mode != kUASheetMode) {
@@ -938,6 +942,7 @@ static bool ValidateSubSelector(const CSSSelector* selector) {
     case CSSSelector::kPseudoHostContext:
     case CSSSelector::kPseudoNot:
     case CSSSelector::kPseudoSpatialNavigationFocus:
+    case CSSSelector::kPseudoSpatialNavigationInterest:
     case CSSSelector::kPseudoIsHtml:
     case CSSSelector::kPseudoListBox:
     case CSSSelector::kPseudoHostHasAppearance:

third_party/blink/renderer/core/css/css_selector.h

@@ -242,6 +242,7 @@ class CORE_EXPORT CSSSelector {
     kPseudoHostContext,
     kPseudoShadow,
     kPseudoSpatialNavigationFocus,
+    kPseudoSpatialNavigationInterest,
     kPseudoIsHtml,
     kPseudoListBox,
     kPseudoHostHasAppearance,

third_party/blink/renderer/core/css/element_rule_collector.cc

@@ -235,9 +235,9 @@ void ElementRuleCollector::CollectMatchingRules(
   if (SelectorChecker::MatchesFocusPseudoClass(element))
     CollectMatchingRulesForList(match_request.rule_set->FocusPseudoClassRules(),
                                 cascade_order, match_request);
-  if (SelectorChecker::MatchesSpatialNavigationFocusPseudoClass(element)) {
+  if (SelectorChecker::MatchesSpatialNavigationInterestPseudoClass(element)) {
     CollectMatchingRulesForList(
-        match_request.rule_set->SpatialNavigationFocusPseudoClassRules(),
+        match_request.rule_set->SpatialNavigationInterestPseudoClassRules(),
         cascade_order, match_request);
   }
   AtomicString element_name = matching_ua_rules_

third_party/blink/renderer/core/css/parser/css_selector_parser_test.cc

@@ -340,6 +340,7 @@ TEST(CSSSelectorParserTest, InternalPseudo) {
                               ":-internal-list-box",
                               ":-internal-shadow-host-has-appearance",
                               ":-internal-spatial-navigation-focus",
+                              ":-internal-spatial-navigation-interest",
                               ":-internal-video-persistent",
                               ":-internal-video-persistent-ancestor"};
   for (auto* test_case : test_cases) {

third_party/blink/renderer/core/css/rule_feature_set.cc

@@ -160,6 +160,7 @@ bool SupportsInvalidation(CSSSelector::PseudoType type) {
     case CSSSelector::kPseudoHost:
     case CSSSelector::kPseudoShadow:
     case CSSSelector::kPseudoSpatialNavigationFocus:
+    case CSSSelector::kPseudoSpatialNavigationInterest:
     case CSSSelector::kPseudoIsHtml:
     case CSSSelector::kPseudoListBox:
     case CSSSelector::kPseudoHostHasAppearance:
@@ -539,7 +540,7 @@ InvalidationSet* RuleFeatureSet::InvalidationSetForSimpleSelector(
       case CSSSelector::kPseudoDefined:
       case CSSSelector::kPseudoVideoPersistent:
       case CSSSelector::kPseudoVideoPersistentAncestor:
-      case CSSSelector::kPseudoSpatialNavigationFocus:
+      case CSSSelector::kPseudoSpatialNavigationInterest:
         return &EnsurePseudoInvalidationSet(selector.GetPseudoType(), type,
                                             position);
       case CSSSelector::kPseudoFirstOfType:

third_party/blink/renderer/core/css/rule_set.cc

@@ -135,7 +135,7 @@ static void ExtractSelectorValues(const CSSSelector* selector,
         case CSSSelector::kPseudoPart:
         case CSSSelector::kPseudoHost:
         case CSSSelector::kPseudoHostContext:
-        case CSSSelector::kPseudoSpatialNavigationFocus:
+        case CSSSelector::kPseudoSpatialNavigationInterest:
           pseudo_type = selector->GetPseudoType();
           break;
         case CSSSelector::kPseudoWebKitCustomElement:
@@ -207,8 +207,8 @@ bool RuleSet::FindBestRuleSetAndAdd(const CSSSelector& component,
     case CSSSelector::kPseudoWebkitAnyLink:
       link_pseudo_class_rules_.push_back(rule_data);
       return true;
-    case CSSSelector::kPseudoSpatialNavigationFocus:
-      spatial_navigation_focus_class_rules_.push_back(rule_data);
+    case CSSSelector::kPseudoSpatialNavigationInterest:
+      spatial_navigation_interest_class_rules_.push_back(rule_data);
       return true;
     case CSSSelector::kPseudoFocus:
       focus_pseudo_class_rules_.push_back(rule_data);
@@ -395,7 +395,7 @@ void RuleSet::CompactRules() {
   link_pseudo_class_rules_.ShrinkToFit();
   cue_pseudo_rules_.ShrinkToFit();
   focus_pseudo_class_rules_.ShrinkToFit();
-  spatial_navigation_focus_class_rules_.ShrinkToFit();
+  spatial_navigation_interest_class_rules_.ShrinkToFit();
   universal_rules_.ShrinkToFit();
   shadow_host_rules_.ShrinkToFit();
   page_rules_.ShrinkToFit();
@@ -431,7 +431,7 @@ void RuleSet::Trace(blink::Visitor* visitor) {
   visitor->Trace(link_pseudo_class_rules_);
   visitor->Trace(cue_pseudo_rules_);
   visitor->Trace(focus_pseudo_class_rules_);
-  visitor->Trace(spatial_navigation_focus_class_rules_);
+  visitor->Trace(spatial_navigation_interest_class_rules_);
   visitor->Trace(universal_rules_);
   visitor->Trace(shadow_host_rules_);
   visitor->Trace(page_rules_);

third_party/blink/renderer/core/css/rule_set.h

@@ -215,9 +215,9 @@ class CORE_EXPORT RuleSet : public GarbageCollectedFinalized<RuleSet> {
     return &focus_pseudo_class_rules_;
   }
   const HeapVector<Member<const RuleData>>*
-  SpatialNavigationFocusPseudoClassRules() const {
+  SpatialNavigationInterestPseudoClassRules() const {
     DCHECK(!pending_rules_);
-    return &spatial_navigation_focus_class_rules_;
+    return &spatial_navigation_interest_class_rules_;
   }
   const HeapVector<Member<const RuleData>>* UniversalRules() const {
     DCHECK(!pending_rules_);
@@ -334,7 +334,7 @@ class CORE_EXPORT RuleSet : public GarbageCollectedFinalized<RuleSet> {
   HeapVector<Member<const RuleData>> link_pseudo_class_rules_;
   HeapVector<Member<const RuleData>> cue_pseudo_rules_;
   HeapVector<Member<const RuleData>> focus_pseudo_class_rules_;
-  HeapVector<Member<const RuleData>> spatial_navigation_focus_class_rules_;
+  HeapVector<Member<const RuleData>> spatial_navigation_interest_class_rules_;
   HeapVector<Member<const RuleData>> universal_rules_;
   HeapVector<Member<const RuleData>> shadow_host_rules_;
   HeapVector<Member<const RuleData>> part_pseudo_rules_;

third_party/blink/renderer/core/css/selector_checker.cc

@@ -75,6 +75,12 @@ static bool IsFrameFocused(const Element& element) {
                                                  .FrameIsFocusedAndActive();
 }
 
+static bool MatchesSpatialNavigationFocusPseudoClass(const Element& element) {
+  return IsHTMLOptionElement(element) &&
+         ToHTMLOptionElement(element).SpatialNavigationFocused() &&
+         IsFrameFocused(element);
+}
+
 static bool MatchesListBoxPseudoClass(const Element& element) {
   return IsHTMLSelectElement(element) &&
          !ToHTMLSelectElement(element).UsesMenuList();
@@ -1072,6 +1078,9 @@ bool SelectorChecker::CheckPseudoClass(const SelectorCheckingContext& context,
     case CSSSelector::kPseudoSpatialNavigationFocus:
       DCHECK(is_ua_rule_);
       return MatchesSpatialNavigationFocusPseudoClass(element);
+    case CSSSelector::kPseudoSpatialNavigationInterest:
+      DCHECK(is_ua_rule_);
+      return MatchesSpatialNavigationInterestPseudoClass(element);
     case CSSSelector::kPseudoIsHtml:
       DCHECK(is_ua_rule_);
       return element.GetDocument().IsHTMLDocument();
@@ -1395,27 +1404,20 @@ bool SelectorChecker::MatchesFocusVisiblePseudoClass(const Element& element) {
 }
 
 // static
-bool SelectorChecker::MatchesSpatialNavigationFocusPseudoClass(
+bool SelectorChecker::MatchesSpatialNavigationInterestPseudoClass(
     const Element& element) {
   if (!IsSpatialNavigationEnabled(element.GetDocument().GetFrame()))
     return false;
-  if (RuntimeEnabledFeatures::FocuslessSpatialNavigationEnabled()) {
-    DCHECK(element.GetDocument().GetPage());
-    Element* interested_element = element.GetDocument()
-                                      .GetPage()
-                                      ->GetSpatialNavigationController()
-                                      .GetInterestedElement();
-    return interested_element && *interested_element == element;
-  }
-  if (RuntimeEnabledFeatures::SpatialNavigationForcesOutlineEnabled()) {
-    // TODO(mthiesse): Decouple spatial navigation target from focus, so that
-    // if spat nav is enabled, but not used, we don't override focus ring
-    // behavior on that element.
-    return element.IsFocused() && IsFrameFocused(element);
-  }
-  return IsHTMLOptionElement(element) &&
-         ToHTMLOptionElement(element).SpatialNavigationFocused() &&
-         IsFrameFocused(element);
+
+  if (!RuntimeEnabledFeatures::FocuslessSpatialNavigationEnabled())
+    return false;
+
+  DCHECK(element.GetDocument().GetPage());
+  Element* interested_element = element.GetDocument()
+                                    .GetPage()
+                                    ->GetSpatialNavigationController()
+                                    .GetInterestedElement();
+  return interested_element && *interested_element == element;
 }
 
 }  // namespace blink

third_party/blink/renderer/core/css/selector_checker.h

@@ -153,7 +153,7 @@ class SelectorChecker {
 
   static bool MatchesFocusPseudoClass(const Element&);
   static bool MatchesFocusVisiblePseudoClass(const Element&);
-  static bool MatchesSpatialNavigationFocusPseudoClass(const Element&);
+  static bool MatchesSpatialNavigationInterestPseudoClass(const Element&);
 
  private:
   // Does the work of checking whether the simple selector and element pointed

third_party/blink/renderer/core/html/resources/html.css

@@ -645,8 +645,8 @@ input:disabled, textarea:disabled {
 }
 
 option:-internal-spatial-navigation-focus {
-    outline: black dashed 1px !important;
-    outline-offset: -1px !important;
+    outline: black dashed 1px;
+    outline-offset: -1px;
 }
 
 datalist {
@@ -989,9 +989,9 @@ nobr {
 
 /* states */
 
-:-internal-spatial-navigation-focus {
-    outline: auto 5px -webkit-focus-ring-color !important;
-    box-shadow: none !important
+:-internal-spatial-navigation-interest {
+  outline: auto 5px -webkit-focus-ring-color !important;
+  box-shadow: none !important
 }
 
 :focus {

third_party/blink/renderer/core/inspector/inspector_trace_events.cc

@@ -340,6 +340,7 @@ const char* PseudoTypeToString(CSSSelector::PseudoType pseudo_type) {
     DEFINE_STRING_MAPPING(PseudoShadow)
     DEFINE_STRING_MAPPING(PseudoSlotted)
     DEFINE_STRING_MAPPING(PseudoSpatialNavigationFocus)
+    DEFINE_STRING_MAPPING(PseudoSpatialNavigationInterest)
     DEFINE_STRING_MAPPING(PseudoIsHtml)
     DEFINE_STRING_MAPPING(PseudoListBox)
     DEFINE_STRING_MAPPING(PseudoHostHasAppearance)

third_party/blink/renderer/platform/runtime_enabled_features.json5

@@ -1295,11 +1295,6 @@
       name: "SmoothScrollJSIntervention",
       status: "stable",
     },
-    // When this is enabled (and Spatial Navigation is enabled), overrides of box shadow and outline
-    // on focused elements will be ignored.
-    {
-      name: "SpatialNavigationForcesOutline",
-    },
     // Used as argument in attribute of stable-release functions/interfaces
     // where a runtime-enabled feature name is required for correct IDL syntax.
     // This is a global flag; do not change its status.

third_party/blink/web_tests/VirtualTestSuites

@@ -1024,11 +1024,6 @@
     "base": "http/tests/streams",
     "args": ["--enable-blink-features=StreamsNative", "--enable-features=BlinkHeapUnifiedGarbageCollection"]
   },
-  {
-    "prefix": "spatial-navigation-force-outline",
-    "base": "fast/spatial-navigation",
-    "args": ["--enable-blink-features=SpatialNavigationForcesOutline"]
-  },
   {
     "prefix": "bidi-caret-affinity",
     "base": "editing/selection/modify_move",

third_party/blink/web_tests/fast/spatial-navigation/snav-focusless-interested-element-indicated-expected.html

@@ -13,5 +13,5 @@
 <div id="third" tabindex="0">Third</div>
 
 <script>
-  document.getElementById("second").focus();
+  document.getElementById("second").style.cssText = "outline: auto 5px -webkit-focus-ring-color !important; box-shadow: none !important";
 </script>

third_party/blink/web_tests/virtual/spatial-navigation-force-outline/fast/spatial-navigation/README.txt

-# This suite tests modifications to the spatial navigation implementation
-# under development.
-# It sets "--enable-blink-features=SpatialNavigationForcesOutline"

third_party/blink/web_tests/virtual/spatial-navigation-force-outline/fast/spatial-navigation/snav-force-focusring.html

-<!DOCTYPE html>
-<body>
-<style>
-:focus {
-  outline: 0;
-  box-shadow: 0 0 2px green;
-}
-</style>
-<p><a id="start" href="#link1">Link 1</a></p>
-<p><a id="end" href="#link2">Link 2</a></p>
-
-<script>
-if (window.testRunner) {
-  testRunner.overridePreference("WebKitTabToLinksPreferenceKey", 1);
-  testRunner.overridePreference("WebKitSpatialNavigationEnabled", 1);
-}
-
-if (window.eventSender) {
-  // Start at a known place.
-  document.getElementById("start").focus();
-  // Focus should move to the second link, and focus ring should be be the
-  // default focus ring (not hidden), and without the green box-shadow.
-  eventSender.keyDown('ArrowDown');
-}
-
-</script>
-</body>