[selectors4] Use common ancestor strategy for :focus-within

third_party/WebKit/Source/core/dom/ContainerNode.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2013 Apple Inc. All rights
  * reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
@@ skipping 1026 matching lines @@
     SetNeedsStyleRecalc(change_type,
                         StyleChangeReasonForTracing::CreateWithExtraData(
                             StyleChangeReason::kPseudoClass,
                             StyleChangeExtraData::g_focus_within));
   }
   if (IsElementNode() &&
       ToElement(this)->ChildrenOrSiblingsAffectedByFocusWithin())
     ToElement(this)->PseudoStateChanged(CSSSelector::kPseudoFocusWithin);
 }
 
-void ContainerNode::SetFocused(bool received, WebFocusType focus_type) {
+void ContainerNode::SetFocused(bool received,
+                               WebFocusType focus_type,
+                               Node* common_ancestor) {
   // Recurse up author shadow trees to mark shadow hosts if it matches :focus.
   // TODO(kochi): Handle UA shadows which marks multiple nodes as focused such
   // as <input type="date"> the same way as author shadow.
   if (ShadowRoot* root = ContainingShadowRoot()) {
     if (root->GetType() != ShadowRootType::kUserAgent)
       OwnerShadowHost()->SetFocused(received, focus_type);

[rune, 2017/04/20 09:22:17]

Having a default argument for common_ancestor looks scary to me. Here we pass
nullptr as the ancestor, right?

Wouldn't it be better if we updated the :focus-within state from
Document::SetFocusedElement()?

Is the problem that focus, hence focus-within, state can change without the
focus element being changed because the whole window loses focus?

[Manuel Rego, 2017/04/20 15:20:37]

Yeah, this is an oversight in this version of the patch,
as I was trying the other way around before.
That was my original idea, but I found that strange problem
when the whole window lose focus. [1]
And I thought there might be more cases where ContainerNode::SetFocused()
was called directly bypassing Document::SetFocusedElement().

But checking the result of this first patch I agree with you that
it's not looking good. :(

I'm uploading a new version, modifying Document::SetFocusedElement(),
which also needs some small changes in FocusController.
Let's see what you think about that.

Of course, any other idea is welcomed.

[1]
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/page/Focu...

   }
 
   // If this is an author shadow host and indirectly focused (has focused
   // element within its shadow root), update focus.
   if (IsElementNode() && GetDocument().FocusedElement() &&
       GetDocument().FocusedElement() != this) {
     if (ToElement(this)->AuthorShadowRoot())
       received =
           received && ToElement(this)->AuthorShadowRoot()->delegatesFocus();
   }
 
   if (IsFocused() == received)
     return;
 
   Node::SetFocused(received, focus_type);
 
   FocusStateChanged();
 
   UpdateDistribution();
-  for (ContainerNode* node = this; node;
+  for (ContainerNode* node = this; node && node != common_ancestor;
        node = FlatTreeTraversal::Parent(*node)) {
     node->SetHasFocusWithin(received);
     node->FocusWithinStateChanged();
   }
 
   if (GetLayoutObject() || received)
     return;
 
   // If :focus sets display: none, we lose focus but still need to recalc our
   // style.
@@ skipping 435 matching lines @@
     return true;
 
   if (node->IsElementNode() && ToElement(node)->Shadow())
     return true;
 
   return false;
 }
 #endif
 
 }  // namespace blink