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

Ryosuke Niwa
Reported 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.
Attachments
Cleanup (13.42 KB, patch)
2016-05-06 00:04 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Renamed one more function (14.69 KB, patch)
2016-05-06 00:18 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2016-05-06 00:04:38 PDT
Created attachment 278240 [details] Cleanup
Ryosuke Niwa
Comment 2 2016-05-06 00:18:29 PDT
Created attachment 278243 [details] Renamed one more function
Darin Adler
Comment 3 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.
Ryosuke Niwa
Comment 4 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.
WebKit Commit Bot
Comment 5 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>
WebKit Commit Bot
Comment 6 2016-05-09 09:19:39 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.