Bug 154660

Created attachment 272162 [details] Cleanup

Created attachment 272163 [details] Removed a tab in change log

Comment on attachment 272163 [details] Removed a tab in change log View in context: https://bugs.webkit.org/attachment.cgi?id=272163&action=review > Source/WebCore/page/FocusController.cpp:118 > -static Node* parentInScope(const Node* node) > -{ > - if (node->isShadowRoot()) > - return nullptr; > - > - ContainerNode* parent = node->parentNode(); > - if (parent && parent->shadowRoot()) > - return nullptr; > - > - return parent; > -} > - > -Node* FocusNavigationScope::nextInScope(const Node* node) const > +Node* FocusNavigationScope::nextInScope(const Node& node) const > { > if (Node* next = firstChildInScope(node)) > return next; > - if (Node* next = node->nextSibling()) > - return next; > - const Node* current = node; > - while (current && !current->nextSibling()) > - current = parentInScope(current); > - return current ? current->nextSibling() : nullptr; > + return NodeTraversal::nextSkippingChildren(node); > } This changes behavior of these functions for children of shadow hosts. Why is that ok?

Comment on attachment 272163 [details] Removed a tab in change log View in context: https://bugs.webkit.org/attachment.cgi?id=272163&action=review >> Source/WebCore/page/FocusController.cpp:118 >> } > > This changes behavior of these functions for children of shadow hosts. Why is that ok? That's a good point :(

Comment on attachment 272163 [details] Removed a tab in change log View in context: https://bugs.webkit.org/attachment.cgi?id=272163&action=review > Source/WebCore/page/FocusController.cpp:85 > + Node* lastLeafNode() const > + { > + if (!m_lastLeafCache) > + findLastLeafNode(); > + return m_lastLeafCache; > + } Does this function body really need to be in the class definition? Makes it harder to read it over. Lets just define this inline function right afterward. >>> Source/WebCore/page/FocusController.cpp:118 >>> } >> >> This changes behavior of these functions for children of shadow hosts. Why is that ok? > > That's a good point :( So marking review- because of this.