Bug 157415

Summary: Refactor FocusController::findFocusableElementRecursively
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: UI EventsAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, darin, kling, koivisto, sam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 151379    
Attachments:
Description Flags
Cleanup
none
Renamed one more function none

Description Ryosuke Niwa 2016-05-05 23:01:12 PDT
FocusController::findFocusableElementRecursively and FocusController::nextFocusableElement/previousFocusableElement are named backwards.
The former finds a focusable element and the latter two finds a focusable element or a shadow host.

Fix this mess.
Comment 1 Ryosuke Niwa 2016-05-06 00:04:38 PDT
Created attachment 278240 [details]
Cleanup
Comment 2 Ryosuke Niwa 2016-05-06 00:18:29 PDT
Created attachment 278243 [details]
Renamed one more function
Comment 3 Darin Adler 2016-05-07 12:50:14 PDT
Comment on attachment 278243 [details]
Renamed one more function

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

I am getting lost in all these super-long function names. The new names do seem more accurate and precise.

> Source/WebCore/page/FocusController.cpp:435
> +    while (owner) {

This would be more readable as a for loop, but I guess it’s hard to get outerScope into something visible in the loop condition.
Comment 4 Ryosuke Niwa 2016-05-09 08:42:47 PDT
Thanks for the review!

(In reply to comment #3)
> Comment on attachment 278243 [details]
> Renamed one more function
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=278243&action=review
> 
> I am getting lost in all these super-long function names. The new names do
> seem more accurate and precise.

Hopefully next round of cleanup will help.

> > Source/WebCore/page/FocusController.cpp:435
> > +    while (owner) {
> 
> This would be more readable as a for loop, but I guess it’s hard to get
> outerScope into something visible in the loop condition.

Yeah, I thought about the same thing but I didn't want to move FocusNavigationScope out of the loop.
Comment 5 WebKit Commit Bot 2016-05-09 09:19:34 PDT
Comment on attachment 278243 [details]
Renamed one more function

Clearing flags on attachment: 278243

Committed r200576: <http://trac.webkit.org/changeset/200576>
Comment 6 WebKit Commit Bot 2016-05-09 09:19:39 PDT
All reviewed patches have been landed.  Closing bug.