Created attachment 26143 [details] add directional navigation to elements

Comment on attachment 26143 [details] add directional navigation to elements > +#if !defined(ENABLE_DPAD_NAVIGATION) > +#define ENABLE_DPAD_NAVIGATION 0 > +#endif I don't think DPAD is an abbreviation for "directional pad" that everyone looking at WebKit would recognize, so please use ENABLE_DIRECTIONAL_PAD_NAVIGATION. > @@ -196,6 +196,7 @@ bool HTMLInputElement::isKeyboardFocusab > if (name().isEmpty()) > return false; > > +if !ENABLE(DPAD_NAVIGATION) > // Never allow keyboard tabbing to leave you in the same radio group. Always > // skip any other elements in the group. > Node* currentFocusedNode = document()->focusedNode(); > @@ -205,6 +206,7 @@ bool HTMLInputElement::isKeyboardFocusab > focusedInput->name() == name()) > return false; > } > +#endif > > // Allow keyboard focus if we're checked or if nothing in the group is checked. > return checked() || !checkedRadioButtons(this).checkedButtonForGroup(name()); I don't understand the connection between a "directional pad" and this change, and the change log comments don't make it clear either. Is the tab key considered to be input from the directional pad? If so, is that the best design? Maybe this should not be a compile time setting, but rather a different event, related to the tab key but not identical. The reason isKeyboardFocusable gets a KeyboardEvent argument is so that different types of keyboard navigation can support different rules about what's skipped. I don't think this change is designed correctly. > +#if ENABLE(DPAD_NAVIGATION) > + if (!checked()) // allow enter to set state of radio > + clickElement = true; > + break; > +#else > break; // Don't do anything for enter on a radio button. > +#endif Same comment here. I think the directional pad Enter event should contain something in it that makes it different at runtime. Just compiling in different behavior for all Enter events doesn't seem like the right design. Looking at the other two changes, I have exactly the same thought. This patch seems to be a set of behavior changes at compile time that should instead be tied to some difference in the directional pad events. Or maybe you could convince me otherwise. review- for now

this code is obsolete in the android tree and has been removed; please ignore this patch request