Bug 61839 - Refactor functions related with focus in WebCore::Document
Summary: Refactor functions related with focus in WebCore::Document
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hayato Ito
URL:
Keywords:
Depends on:
Blocks: 61410
  Show dependency tree / graph
 
Reported: 2011-06-01 00:36 PDT by Hayato Ito
Modified: 2011-06-01 19:24 PDT (History)
4 users (show)

See Also:

Attachments
move focus related functions from Document to FocusController (18.04 KB, patch)
2011-06-01 00:40 PDT, Hayato Ito 		tkent: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Hayato Ito 2011-06-01 00:36:00 PDT 
This is a separated patch form bug 61410, containing only a refactoring part.
Comment 1 Hayato Ito 2011-06-01 00:40:14 PDT 
Created attachment 95558 [details]
move focus related functions from Document to FocusController
Comment 2 Kent Tamura 2011-06-01 01:01:53 PDT 
Comment on attachment 95558 [details]
move focus related functions from Document to FocusController

View in context: https://bugs.webkit.org/attachment.cgi?id=95558&action=review

> Source/WebCore/page/FocusController.cpp:312
> +    for (Node* n = start; n; n = n->traverseNextNode())

nit: Should avoid 1-letter variable name.

> Source/WebCore/page/FocusController.cpp:332
> +    int winningTabIndex = SHRT_MAX + 1;

nit: Because this is C++, we had better use numeric_limits<short> instead of SHRT_MAX.

> Source/WebCore/page/FocusController.h:79
> +    /**

nit: We usually use '//' for comments.
Comment 3 Hayato Ito 2011-06-01 02:03:02 PDT 
Thank you for the review. I'll land this patch after addressing the review comments.

(In reply to comment #2)
> (From update of attachment 95558 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=95558&action=review
> 
> > Source/WebCore/page/FocusController.cpp:312
> > +    for (Node* n = start; n; n = n->traverseNextNode())
> 
> nit: Should avoid 1-letter variable name.
> 
> > Source/WebCore/page/FocusController.cpp:332
> > +    int winningTabIndex = SHRT_MAX + 1;
> 
> nit: Because this is C++, we had better use numeric_limits<short> instead of SHRT_MAX.
> 
> > Source/WebCore/page/FocusController.h:79
> > +    /**
> 
> nit: We usually use '//' for comments.
Comment 4 Hayato Ito 2011-06-01 19:24:11 PDT 
Committed r87874: <http://trac.webkit.org/changeset/87874>