Bug 116676 – [BlackBerry] Respect tabindex when using form controls.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit BlackBerry

[Mike Fenton]

SSIA.

[Mike Fenton]

Created attachment 202715 [details]
Patch

[Xan Lopez]

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.

[Mike Fenton]

Created attachment 202726 [details]
Updated Patch

[Xan Lopez]

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.

[WebKit Commit Bot]

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

[Mike Fenton]

Created attachment 202805 [details]
Fixed patch.

[WebKit Commit Bot]

Comment on attachment 202805 [details]
Fixed patch.

Clearing flags on attachment: 202805

Committed r150637: <http://trac.webkit.org/changeset/150637>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.