Summary: | [BlackBerry] Respect tabindex when using form controls. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mike Fenton <mifenton> | ||||||||
Component: | WebKit BlackBerry | Assignee: | Mike Fenton <mifenton> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Mike Fenton
2013-05-23 08:01:03 PDT
Created attachment 202715 [details]
Patch
Comment on attachment 202715 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=202715&action=review It took me a while to figure out what this was doing. I think here it would be really useful to have a general comment at the beginning saying that we basically: Do a full pass for every node. The main case is the focus node, there we use the source order to select the prev/next candidates. For the other nodes, if tabIndex is positive, we use their tabIndex to pick the best prev/next. Otherwise remember the highest tabindex, since that will be the prev node whe tabIndex of the focused node is 0. This is pretty hairy, I'm sure you can write a good explanation. The logic seems sound to me after examination, but r- to add a good comment. > Source/WebKit/blackberry/WebKitSupport/InputHandler.cpp:956 > + int nextTabIndex = std::numeric_limits<short>::max() + 1; This seems pretty magical at first sight, since numeric limit for short + 1 in an int is just A Big Number(tm). You mentioned on IRC that this is likely because the limit for tab index is a short, so maybe worth having a comment. Created attachment 202726 [details]
Updated Patch
Comment on attachment 202726 [details]
Updated Patch
Looks good to me, but EWS says the patch does not apply cleanly, so I think we won't be able to land it. r+ anyway.
Comment on attachment 202726 [details] Updated Patch Rejecting attachment 202726 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 202726, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: z']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Parsed 2 diffs from patch file(s). patching file Source/WebKit/blackberry/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebKit/blackberry/WebKitSupport/InputHandler.cpp patch: **** malformed patch at line 124: + m_previousFocusableTextElement = highestTabIndexElement; Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Xan Lopez']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Full output: http://webkit-queues.appspot.com/results/686021 Created attachment 202805 [details]
Fixed patch.
Comment on attachment 202805 [details] Fixed patch. Clearing flags on attachment: 202805 Committed r150637: <http://trac.webkit.org/changeset/150637> All reviewed patches have been landed. Closing bug. |