[webkit-reviews] review denied: [Bug 116676] [BlackBerry] Respect tabindex when using form controls. : [Attachment 202715] Patch

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has denied Mike Fenton
<mifenton at blackberry.com>'s request for review:
Bug 116676: [BlackBerry] Respect tabindex when using form controls.
https://bugs.webkit.org/show_bug.cgi?id=116676

Attachment 202715: Patch
https://bugs.webkit.org/attachment.cgi?id=202715&action=review

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
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.