[Webkit-unassigned] [Bug 117265] Spatial Navigation should avoid unwanted calculation while deciding focus candidate.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=117265


Antonio Gomes <tonikitoo at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #204149|review?                     |review-
               Flag|                            |




--- Comment #8 from Antonio Gomes <tonikitoo at webkit.org>  2013-06-10 06:25:15 PST ---
(From update of attachment 204149)
View in context: https://bugs.webkit.org/attachment.cgi?id=204149&action=review

> LayoutTests/fast/spatial-navigation/snav-search-optimization.html:6
> +    1) DRT support for SNav enable/disable.

DRT = TestRunner now

> LayoutTests/fast/spatial-navigation/snav-search-optimization.html:38
> +        shouldBe("internals.numberOfProcessedNodes(document)", "4");         //current focused node is 5 and navigation direction is DOWN

I think you should also test that the currently focused node is '5'. Same the the below checks...

> Source/WebCore/testing/Internals.cpp:393
> +unsigned Internals::numberOfProcessedNodes(Document* document, ExceptionCode& ec) const
> +{

since you are passing a "document" you should also have a test involving inner frame.

Specially becaused an inner frame's focusController is different from its parents.

> Source/WebCore/testing/Internals.h:98
> +    unsigned numberOfProcessedNodes(Document*, ExceptionCode&) const;

It is a way too generic name. r- due to that.

Who is reading this code "focusController->numberOfProcessedNodes" will never imagine it is related to Spatial Nav.

maybe lastSpatialNavigationCandidatesCount or something like this.

> Source/WebCore/page/SpatialNavigation.cpp:630
> +            result = true;

return true directly here. Same for the other clauses below.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.