[webkit-reviews] review granted: [Bug 36168] Spatial Navigation: Code simplification in FocusController.cpp and SpatialNavigation.cpp : [Attachment 51940] patch_2 - v4 - code simplification

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 2 15:21:49 PDT 2010


Simon Fraser (smfr) <simon.fraser at apple.com> has granted Antonio Gomes
(tonikitoo) <tonikitoo at webkit.org>'s request for review:
Bug 36168: Spatial Navigation: Code simplification in FocusController.cpp and
SpatialNavigation.cpp
https://bugs.webkit.org/show_bug.cgi?id=36168

Attachment 51940: patch_2 - v4 - code simplification
https://bugs.webkit.org/attachment.cgi?id=51940&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog

> +	   data, while all assignement logic previously distanceInDirection
method

previously _in_

> +	   Parent document distance and alignment acquisitions, on their turns,
have

s/on their turns/in turn

> +	   also changed location: they are both got from
deepFindFocusableNodeInDirection,

s/got from/obtained from

> +	   and passed in a recursive call to findFocusableNodeInDirection via
the candidateParent
> +	   variable (options parameter). Yet on
deepFindFocusableNodeInDirection method, the need for the

s/Yet/In addition,


> diff --git a/WebCore/page/FocusController.h b/WebCore/page/FocusController.h

> +    void findFocusableNodeInDirection(Document*, Node*, FocusDirection,
KeyboardEvent*, FocusCandidate&, const FocusCandidate& c = FocusCandidate());

I think you should provide names for the two FocusCandidate arguments so it's
possible to tell what they are for.

r=me


More information about the webkit-reviews mailing list