[webkit-reviews] review granted: [Bug 78588] Focus navigation should traverse nodes in reified DOM tree order. : [Attachment 134229] update

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has granted Hayato Ito
<hayato at chromium.org>'s request for review:
Bug 78588: Focus navigation should traverse nodes in reified DOM tree order.
https://bugs.webkit.org/show_bug.cgi?id=78588

Attachment 134229: update
https://bugs.webkit.org/attachment.cgi?id=134229&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=134229&action=review


> Source/WebCore/page/FocusController.cpp:298
> +    RefPtr<Node> node = findFocusableNodeAcrossFocusScope(direction,
currentNode ? FocusScope::focusScopeOf(currentNode) :
FocusScope::focusScopeOf(document), currentNode, event);

The argument looks like it should be written:
FocusScope::focusScopeOf(currentNode ? currentNode : document).

> Source/WebCore/page/FocusController.cpp:372
> +	   if (foundInInnerFocusScope)
> +	       found = foundInInnerFocusScope;
> +	   else
> +	       found = findFocusableNodeRecursively(direction, scope,
currentNode, event);

Also looks like it could be an inline if.

> Source/WebCore/page/FocusController.cpp:394
> +    // start node is exclusive.

Capitalize first word, since it's a sentence.

> Source/WebCore/page/FocusController.cpp:404
> +	   if (foundInInnerFocusScope)
> +	       return foundInInnerFocusScope;
> +	   return findFocusableNodeRecursively(direction, scope, found, event);


I think you hate inline ifs :P