Bug 22935 - navigate elements with directional pad
Summary: navigate elements with directional pad
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-12-19 08:27 PST by Cary Clark
Modified: 2009-01-06 11:05 PST (History)
1 user (show)

See Also:


Attachments
add directional navigation to elements (4.53 KB, patch)
2008-12-19 08:29 PST, Cary Clark
darin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Cary Clark 2008-12-19 08:27:17 PST
A mobile device may have only a directional pad to navigate between links. Allow input and select elements to navigate between entries and radio buttons using the directional pad. Also, if dpad navigation is enabled, allow 'Enter' to set the radio's state.
Comment 1 Cary Clark 2008-12-19 08:29:36 PST
Created attachment 26143 [details]
add directional navigation to elements
Comment 2 Darin Adler 2009-01-02 11:17:13 PST
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
Comment 3 Cary Clark 2009-01-06 11:05:13 PST
this code is obsolete in the android tree and has been removed; please ignore this patch request