[Webkit-unassigned] [Bug 22935] navigate elements with directional pad

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=22935


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #26143|review?                     |review-
               Flag|                            |




------- Comment #2 from darin at apple.com  2009-01-02 11:17 PDT -------
(From update of attachment 26143)
> +#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


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.