Changeset 200576 in webkit


Ignore:
Timestamp:
May 9, 2016 9:20:20 AM (8 years ago)
Author:
rniwa@webkit.org
Message:

Refactor FocusController::findFocusableElementRecursively
https://bugs.webkit.org/show_bug.cgi?id=157415

Reviewed by Darin Adler.

Refactor FocusController::findFocusableElementRecursively and related functions. Extracted two functions:
nextFocusableElementWithinScope and previousFocusableElementWithinScope out of it since they didn't really share
any code other than calling findFocusableElement at the beginning.

Also renamed internal variant of nextFocusableElement and previousFocusableElement to nextFocusableElementOrScopeOwner
and previousFocusableElementOrScopeOwner. It was confusing to have these internal functions in addition to public
member functions that are used in Objective-C DOM API.

No new tests are added since there should be no behavioral change.

  • page/FocusController.cpp:

(WebCore::FocusController::findFocusableElementDescendingDownIntoFrameDocument): Added a FIXME.
(WebCore::FocusController::advanceFocusInDocumentOrder): Use findFocusableElementAcrossFocusScope instead of manually
calling findFocusableElementRecursively and findFocusableElementDescendingDownIntoFrameDocument separately.
(WebCore::FocusController::findFocusableElementAcrossFocusScope): Now that findFocusableElementWithinScope calls
findFocusableElementDescendingDownIntoFrameDocument internally, there is no need to keep around "found" local variable.
Introduce a few early exists for a better clarity.
(WebCore::FocusController::findFocusableElementWithinScope): Renamed from findFocusableElementRecursively. Also call
findFocusableElementDescendingDownIntoFrameDocument here instead of findFocusableElementAcrossFocusScope.
(WebCore::FocusController::nextFocusableElementWithinScope): Extracted from findFocusableElementRecursively.
(WebCore::FocusController::previousFocusableElementWithinScope): Ditto.
(WebCore::FocusController::findFocusableElement):
(WebCore::FocusController::nextFocusableElement): Added a FIXME.
(WebCore::FocusController::previousFocusableElement): Ditto.
(WebCore::FocusController::findFocusableElementOrScopeOwner): Renamed from findFocusableElement for clarity.
(WebCore::FocusController::nextFocusableElementOrScopeOwner): Ditto from nextFocusableElement.
(WebCore::FocusController::previousFocusableElementOrScopeOwner): Ditto from nextFocusableElement.

  • page/FocusController.h:
Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r200572 r200576  
     12016-05-09  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Refactor FocusController::findFocusableElementRecursively
     4        https://bugs.webkit.org/show_bug.cgi?id=157415
     5
     6        Reviewed by Darin Adler.
     7
     8        Refactor FocusController::findFocusableElementRecursively and related functions. Extracted two functions:
     9        nextFocusableElementWithinScope and previousFocusableElementWithinScope out of it since they didn't really share
     10        any code other than calling findFocusableElement at the beginning.
     11
     12        Also renamed internal variant of nextFocusableElement and previousFocusableElement to nextFocusableElementOrScopeOwner
     13        and previousFocusableElementOrScopeOwner. It was confusing to have these internal functions in addition to public
     14        member functions that are used in Objective-C DOM API.
     15
     16        No new tests are added since there should be no behavioral change.
     17
     18        * page/FocusController.cpp:
     19        (WebCore::FocusController::findFocusableElementDescendingDownIntoFrameDocument): Added a FIXME.
     20        (WebCore::FocusController::advanceFocusInDocumentOrder): Use findFocusableElementAcrossFocusScope instead of manually
     21        calling findFocusableElementRecursively and findFocusableElementDescendingDownIntoFrameDocument separately.
     22        (WebCore::FocusController::findFocusableElementAcrossFocusScope): Now that findFocusableElementWithinScope calls
     23        findFocusableElementDescendingDownIntoFrameDocument internally, there is no need to keep around "found" local variable.
     24        Introduce a few early exists for a better clarity.
     25        (WebCore::FocusController::findFocusableElementWithinScope): Renamed from findFocusableElementRecursively. Also call
     26        findFocusableElementDescendingDownIntoFrameDocument here instead of findFocusableElementAcrossFocusScope.
     27        (WebCore::FocusController::nextFocusableElementWithinScope): Extracted from findFocusableElementRecursively.
     28        (WebCore::FocusController::previousFocusableElementWithinScope): Ditto.
     29        (WebCore::FocusController::findFocusableElement):
     30        (WebCore::FocusController::nextFocusableElement): Added a FIXME.
     31        (WebCore::FocusController::previousFocusableElement): Ditto.
     32        (WebCore::FocusController::findFocusableElementOrScopeOwner): Renamed from findFocusableElement for clarity.
     33        (WebCore::FocusController::nextFocusableElementOrScopeOwner): Ditto from nextFocusableElement.
     34        (WebCore::FocusController::previousFocusableElementOrScopeOwner): Ditto from nextFocusableElement.
     35        * page/FocusController.h:
     36
    1372016-05-09  Manuel Rego Casasnovas  <rego@igalia.com>
    238
  • trunk/Source/WebCore/page/FocusController.cpp

    r200540 r200576  
    301301        if (!owner.contentFrame())
    302302            break;
    303         Element* foundElement = findFocusableElement(direction, FocusNavigationScope::scopeOwnedByIFrame(owner), 0, event);
     303        // FIXME: This can return a non-focusable shadow root.
     304        Element* foundElement = findFocusableElementOrScopeOwner(direction, FocusNavigationScope::scopeOwnedByIFrame(owner), 0, event);
    304305        if (!foundElement)
    305306            break;
     
    367368
    368369        // Chrome doesn't want focus, so we should wrap focus.
    369         element = findFocusableElementRecursively(direction, FocusNavigationScope::scopeOf(*m_page.mainFrame().document()), 0, event);
    370         element = findFocusableElementDescendingDownIntoFrameDocument(direction, element.get(), event);
     370        element = findFocusableElementAcrossFocusScope(direction, FocusNavigationScope::scopeOf(*m_page.mainFrame().document()), nullptr, event);
    371371
    372372        if (!element)
     
    422422{
    423423    ASSERT(!is<Element>(currentNode) || !isNonFocusableShadowHost(*downcast<Element>(currentNode), *event));
    424     Element* found;
     424
    425425    if (currentNode && direction == FocusDirectionForward && isFocusableShadowHost(*currentNode, *event)) {
    426         Element* foundInInnerFocusScope = findFocusableElementRecursively(direction, FocusNavigationScope::scopeOwnedByShadowHost(downcast<Element>(*currentNode)), 0, event);
    427         found = foundInInnerFocusScope ? foundInInnerFocusScope : findFocusableElementRecursively(direction, scope, currentNode, event);
    428     } else
    429         found = findFocusableElementRecursively(direction, scope, currentNode, event);
     426        if (Element* candidateInInnerScope = findFocusableElementWithinScope(direction, FocusNavigationScope::scopeOwnedByShadowHost(downcast<Element>(*currentNode)), 0, event))
     427            return candidateInInnerScope;
     428    }
     429
     430    if (Element* candidateInCurrentScope = findFocusableElementWithinScope(direction, scope, currentNode, event))
     431        return candidateInCurrentScope;
    430432
    431433    // If there's no focusable node to advance to, move up the focus scopes until we find one.
    432434    Element* owner = scope.owner();
    433     while (!found && owner) {
    434         FocusNavigationScope currentScope = FocusNavigationScope::scopeOf(*owner);
    435         if (direction == FocusDirectionBackward && isFocusableShadowHost(*owner, *event)) {
    436             found = owner;
    437             break;
    438         }
    439         found = findFocusableElementRecursively(direction, currentScope, owner, event);
    440         owner = currentScope.owner();
    441     }
    442     found = findFocusableElementDescendingDownIntoFrameDocument(direction, found, event);
    443     return found;
    444 }
    445 
    446 Element* FocusController::findFocusableElementRecursively(FocusDirection direction, const FocusNavigationScope& scope, Node* start, KeyboardEvent* event)
     435    while (owner) {
     436        FocusNavigationScope outerScope = FocusNavigationScope::scopeOf(*owner);
     437        if (direction == FocusDirectionBackward && isFocusableShadowHost(*owner, *event))
     438            return findFocusableElementDescendingDownIntoFrameDocument(direction, owner, event);
     439        if (Element* candidateInOuterScope = findFocusableElementWithinScope(direction, outerScope, owner, event))
     440            return candidateInOuterScope;
     441        owner = outerScope.owner();
     442    }
     443    return nullptr;
     444}
     445
     446Element* FocusController::findFocusableElementWithinScope(FocusDirection direction, const FocusNavigationScope& scope, Node* start, KeyboardEvent* event)
    447447{
    448448    // Starting node is exclusive.
    449     Element* found = findFocusableElement(direction, scope, start, event);
     449    Element* candidate = direction == FocusDirectionForward
     450        ? nextFocusableElementWithinScope(scope, start, event)
     451        : previousFocusableElementWithinScope(scope, start, event);
     452    return findFocusableElementDescendingDownIntoFrameDocument(direction, candidate, event);
     453}
     454
     455Element* FocusController::nextFocusableElementWithinScope(const FocusNavigationScope& scope, Node* start, KeyboardEvent* event)
     456{
     457    Element* found = nextFocusableElementOrScopeOwner(scope, start, event);
    450458    if (!found)
    451459        return nullptr;
    452     if (direction == FocusDirectionForward) {
    453         if (!isNonFocusableShadowHost(*found, *event))
    454             return found;
    455         Element* foundInInnerFocusScope = findFocusableElementRecursively(direction, FocusNavigationScope::scopeOwnedByShadowHost(*found), 0, event);
    456         return foundInInnerFocusScope ? foundInInnerFocusScope : findFocusableElementRecursively(direction, scope, found, event);
    457     }
    458     ASSERT(direction == FocusDirectionBackward);
     460    if (isNonFocusableShadowHost(*found, *event)) {
     461        if (Element* foundInInnerFocusScope = nextFocusableElementWithinScope(FocusNavigationScope::scopeOwnedByShadowHost(*found), 0, event))
     462            return foundInInnerFocusScope;
     463        return nextFocusableElementWithinScope(scope, found, event);
     464    }
     465    return found;
     466}
     467
     468Element* FocusController::previousFocusableElementWithinScope(const FocusNavigationScope& scope, Node* start, KeyboardEvent* event)
     469{
     470    Element* found = previousFocusableElementOrScopeOwner(scope, start, event);
     471    if (!found)
     472        return nullptr;
    459473    if (isFocusableShadowHost(*found, *event)) {
    460         Element* foundInInnerFocusScope = findFocusableElementRecursively(direction, FocusNavigationScope::scopeOwnedByShadowHost(*found), 0, event);
    461         return foundInInnerFocusScope ? foundInInnerFocusScope : found;
     474        // Search an inner focusable element in the shadow tree from the end.
     475        if (Element* foundInInnerFocusScope = previousFocusableElementWithinScope(FocusNavigationScope::scopeOwnedByShadowHost(*found), 0, event))
     476            return foundInInnerFocusScope;
     477        return found;
    462478    }
    463479    if (isNonFocusableShadowHost(*found, *event)) {
    464         Element* foundInInnerFocusScope = findFocusableElementRecursively(direction, FocusNavigationScope::scopeOwnedByShadowHost(*found), 0, event);
    465         return foundInInnerFocusScope ? foundInInnerFocusScope :findFocusableElementRecursively(direction, scope, found, event);
     480        if (Element* foundInInnerFocusScope = previousFocusableElementWithinScope(FocusNavigationScope::scopeOwnedByShadowHost(*found), 0, event))
     481            return foundInInnerFocusScope;
     482        return previousFocusableElementWithinScope(scope, found, event);
    466483    }
    467484    return found;
    468485}
    469486
    470 Element* FocusController::findFocusableElement(FocusDirection direction, const FocusNavigationScope& scope, Node* node, KeyboardEvent* event)
     487Element* FocusController::findFocusableElementOrScopeOwner(FocusDirection direction, const FocusNavigationScope& scope, Node* node, KeyboardEvent* event)
    471488{
    472489    return (direction == FocusDirectionForward)
    473         ? nextFocusableElement(scope, node, event)
    474         : previousFocusableElement(scope, node, event);
     490        ? nextFocusableElementOrScopeOwner(scope, node, event)
     491        : previousFocusableElementOrScopeOwner(scope, node, event);
    475492}
    476493
     
    529546Element* FocusController::nextFocusableElement(Node& start)
    530547{
     548    // FIXME: This can return a non-focusable shadow host.
    531549    Ref<KeyboardEvent> keyEvent = KeyboardEvent::createForDummy();
    532     return nextFocusableElement(FocusNavigationScope::scopeOf(start), &start, keyEvent.ptr());
     550    return nextFocusableElementOrScopeOwner(FocusNavigationScope::scopeOf(start), &start, keyEvent.ptr());
    533551}
    534552
    535553Element* FocusController::previousFocusableElement(Node& start)
    536554{
     555    // FIXME: This can return a non-focusable shadow host.
    537556    Ref<KeyboardEvent> keyEvent = KeyboardEvent::createForDummy();
    538     return previousFocusableElement(FocusNavigationScope::scopeOf(start), &start, keyEvent.ptr());
    539 }
    540 
    541 Element* FocusController::nextFocusableElement(const FocusNavigationScope& scope, Node* start, KeyboardEvent* event)
     557    return previousFocusableElementOrScopeOwner(FocusNavigationScope::scopeOf(start), &start, keyEvent.ptr());
     558}
     559
     560Element* FocusController::nextFocusableElementOrScopeOwner(const FocusNavigationScope& scope, Node* start, KeyboardEvent* event)
    542561{
    543562    int startTabIndex = 0;
     
    576595}
    577596
    578 Element* FocusController::previousFocusableElement(const FocusNavigationScope& scope, Node* start, KeyboardEvent* event)
     597Element* FocusController::previousFocusableElementOrScopeOwner(const FocusNavigationScope& scope, Node* start, KeyboardEvent* event)
    579598{
    580599    Node* last = nullptr;
  • trunk/Source/WebCore/page/FocusController.h

    r197021 r200576  
    9090
    9191    Element* findFocusableElementAcrossFocusScope(FocusDirection, const FocusNavigationScope& startScope, Node* start, KeyboardEvent*);
    92     Element* findFocusableElementRecursively(FocusDirection, const FocusNavigationScope&, Node* start, KeyboardEvent*);
     92
     93    Element* findFocusableElementWithinScope(FocusDirection, const FocusNavigationScope&, Node* start, KeyboardEvent*);
     94    Element* nextFocusableElementWithinScope(const FocusNavigationScope&, Node* start, KeyboardEvent*);
     95    Element* previousFocusableElementWithinScope(const FocusNavigationScope&, Node* start, KeyboardEvent*);
     96
    9397    Element* findFocusableElementDescendingDownIntoFrameDocument(FocusDirection, Element*, KeyboardEvent*);
    9498
     
    102106    //
    103107    // See http://www.w3.org/TR/html4/interact/forms.html#h-17.11.1
    104     Element* findFocusableElement(FocusDirection, const FocusNavigationScope&, Node* start, KeyboardEvent*);
     108    Element* findFocusableElementOrScopeOwner(FocusDirection, const FocusNavigationScope&, Node* start, KeyboardEvent*);
    105109
    106110    Element* findElementWithExactTabIndex(const FocusNavigationScope&, Node* start, int tabIndex, KeyboardEvent*, FocusDirection);
    107111   
    108     Element* nextFocusableElement(const FocusNavigationScope&, Node* start, KeyboardEvent*);
    109     Element* previousFocusableElement(const FocusNavigationScope&, Node* start, KeyboardEvent*);
     112    Element* nextFocusableElementOrScopeOwner(const FocusNavigationScope&, Node* start, KeyboardEvent*);
     113    Element* previousFocusableElementOrScopeOwner(const FocusNavigationScope&, Node* start, KeyboardEvent*);
    110114
    111115    bool advanceFocusDirectionallyInContainer(Node* container, const LayoutRect& startingRect, FocusDirection, KeyboardEvent*);
Note: See TracChangeset for help on using the changeset viewer.