Bug 37134 – Remove unused DIRECTIONAL_PAD_NAVIGATION

• Component: Accessibility
• Status: RESOLVED FIXED

[Antonio Gomes]

bug 23145 was filed intending to add a DIRECTIONAL_PAD_NAVIGATION build flag to WebCore (see attached patch in that bug), but according to https://bugs.webkit.org/show_bug.cgi?id=23145#c3 this is no longer needed once android does not implement ΅directional navigation" in engine side, nor seems to plan so. Bug was then RESOLVED as INVALID.

In the meanwhile, bug 23163 landed a patch that relies on the DIRECTIONAL_PAD_NAVIGATION flag, and it is not dead code.

I propose to remove any refenrece to DIRECTIONAL_PAD_NAVIGATION from trunk

ps: we have "Spatial Navigation" available in trunk (see bug 18862 and dependency list).

[Antonio Gomes]

Created attachment 52601 [details]
(committed in r57148) patch

[Antonio Gomes]

s/it is not dead code/it is NOW dead code/

[Laszlo Gombos]

Bug 18862 does not seems to be related to Spatial Navigation.

ChangeLog has some minor typos, spacing issues, please fix it when landing.

This patch looks ok to me, but I'd like to give some time to Cary to give feedback before giving an r+. I do not think we have enough evidence that this flag is not used by any ports. The fact that this is not listed in Platform.h is not enough evidence.

[Antonio Gomes]

(In reply to comment #3)
> Bug 18862 does not seems to be related to Spatial Navigation.

err, it is bug 18662. Sorry.

> ChangeLog has some minor typos, spacing issues, please fix it when landing.

sure, thank you.

> This patch looks ok to me, but I'd like to give some time to Cary to give
> feedback before giving an r+. I do not think we have enough evidence that this
> flag is not used by any ports. The fact that this is not listed in Platform.h
> is not enough evidence.

cary, eric, what do you guys think?

[Cary Clark]

The DIRECTIONAL_PAD_NAVIGATION condition is obsolete. D-pad navigation on Android used to change the DOM focus; now, it only changes the mouse position. Since the focus is only set when the page initiates Javascript, or through mouse click events, this scroll in response to changing focus no longer needs to be suppressed.

Android separately ignores scroll requests that are not initiated by user actions, so it's fine to remove this.

[Antonio Gomes]

(In reply to comment #5)
> The DIRECTIONAL_PAD_NAVIGATION condition is obsolete. D-pad navigation on
> Android used to change the DOM focus; now, it only changes the mouse position.
> Since the focus is only set when the page initiates Javascript, or through
> mouse click events, this scroll in response to changing focus no longer needs
> to be suppressed.
> 
> Android separately ignores scroll requests that are not initiated by user
> actions, so it's fine to remove this.

thank you Cary. Laszlo, what typos you referred to?

[Laszlo Gombos]

Comment on attachment 52601 [details]
(committed in r57148) patch

LGTM based on Cary's comments. The typo seems to be only in the bug report and not in ChangeLog.

[Antonio Gomes]

Comment on attachment 52601 [details]
(committed in r57148) patch

thanks laszlo