Bug 52262

Summary: More keybindings for input[type=range]
Product: WebKit Reporter: Erik Arvidsson <arv>
Component: FormsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: shinyak, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows 7   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Erik Arvidsson 2011-01-11 16:23:10 PST
At least on Windows the following keys should modify the input type=range element.

For horizontal slider:

Home => value = min
End => value = max
Page-Up => value -= big step
Page-Down => value += big step
Left => value -= step
Right => value += step
Up => value -= step
Down => value += step

For vertical sliders this is flipped since min is at the bottom.
Comment 1 Kent Tamura 2011-01-11 17:31:46 PST
(In reply to comment #0)
> Home => value = min
> End => value = max
> Page-Up => value -= big step
> Page-Down => value += big step
> Left => value -= step
> Right => value += step
> Up => value -= step
> Down => value += step
> 
> For vertical sliders this is flipped since min is at the bottom.

It sounds reasonable.
How much is "big step"?
Comment 2 Erik Arvidsson 2011-01-11 18:32:53 PST
A "big step" is usually something that can be set on the component. HTML5 does not provide this so maybe we should do something like 10% (round to nearest step).
Comment 3 Kent Tamura 2011-01-20 18:27:29 PST
FYI
Opera's type=range implementation supports only cursor keys, and not Home End PageUp Pand ageDown.

OSX's native slider seems to support no keyboard operation (really?)

Windows's native slider control supports only cursor keys, and not Home End PageUp and PageDown.
Comment 4 Erik Arvidsson 2011-01-21 10:19:51 PST
(In reply to comment #3)
> Windows's native slider control supports only cursor keys, and not Home End PageUp and PageDown.

The keys I listed are the ones used in "Volume Mixer" and in "Control Panel\Appearance and Personalization\Personalization\Window Color and Appearance" in Windows 7.
Comment 5 Kent Tamura 2011-01-23 21:36:24 PST
(In reply to comment #4)
> (In reply to comment #3)
> > Windows's native slider control supports only cursor keys, and not Home End PageUp and PageDown.
> 
> The keys I listed are the ones used in "Volume Mixer" and in "Control Panel\Appearance and Personalization\Personalization\Window Color and Appearance" in Windows 7.

You're right.  I confirmed the keys worked in "Volume Mixer" and Control Panel -> Keyboard.
Probably I had checked with a wrong slider (?).
Comment 6 Shinya Kawanaka 2011-07-06 03:51:02 PDT
Created attachment 99809 [details]
Patch
Comment 7 Kent Tamura 2011-07-06 08:07:06 PDT
Comment on attachment 99809 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=99809&action=review

r- because the key operations are not compatible with Windows.

> Source/WebCore/html/RangeInputType.cpp:202
> +    if (key == "Up" || key == "Right")
> +        newValue = current + step;
> +    else if (key == "Down" || key == "Left")
> +        newValue = current - step;
> +    else if (key == "PageDown")
> +        newValue = current + bigStep;
> +    else if (key == "PageUp")
> +        newValue = current - bigStep;
> +    else if (key == "Home")
> +        newValue = minimum();
> +    else if (key == "End")
> +        newValue = maximum();
> +    else
> +        return; // Did not match any key binding.

A horizontal slider in Control panel -> Keyboard in Windows 7 behaves:
 Up: Increase the value
 Down: Decrease the value
 PageUp: Increase the value by a large amount
 PageDown: Decrease the value by a large amount
 Home: Jump to the minimum value
 End: Jump to the maximum value

A vertical slider in Volume mixer in Windows 7 behaves:
 Right: decrease the value
 Left: Increase the value
 PageUp: Increase the value by a large amount
 PageDown: Decrease the value by a large amount
 Home: Jump to the maximum value
 End: Jump to the minimum value

You can detect slider orientation by renderer()->style()->appearance().
Comment 8 Shinya Kawanaka 2011-07-07 00:08:24 PDT
Created attachment 99949 [details]
Patch
Comment 9 Kent Tamura 2011-07-07 00:23:34 PDT
Comment on attachment 99949 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=99949&action=review

> Source/WebCore/html/RangeInputType.cpp:188
> +    bool isVertical = element()->renderer()->style()->appearance() == SliderVerticalPart;

element()->renderer() can be null.
MediaVolumeSliderPart is also vertical.
Comment 10 Shinya Kawanaka 2011-07-07 02:37:56 PDT
Created attachment 99962 [details]
Patch
Comment 11 Kent Tamura 2011-07-07 02:56:19 PDT
Comment on attachment 99962 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=99962&action=review

ok.
Thank you for fixing this.

> LayoutTests/ChangeLog:10
> +        Added more key bindings for input[type=range].
> +        https://bugs.webkit.org/show_bug.cgi?id=52262
> +
> +        Added PageUp/PageDown/Home/End key bindings tests for input[type=range].
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        * fast/forms/range-keyoperation-expected.txt: Regenerated.

The ChangeLog format is:

<summary>
<Bug URL>

<Reviewed by....>

<Detail>

<Files/functions>
Comment 12 WebKit Review Bot 2011-07-07 03:37:53 PDT
Comment on attachment 99962 [details]
Patch

Clearing flags on attachment: 99962

Committed r90552: <http://trac.webkit.org/changeset/90552>
Comment 13 WebKit Review Bot 2011-07-07 03:37:58 PDT
All reviewed patches have been landed.  Closing bug.
Comment 14 Alexey Proskuryakov 2011-07-08 13:58:05 PDT
Could this patch break <platform/mac/accessibility/html-slider-indicator.html>?

See <http://build.webkit.org/results/SnowLeopard%20Intel%20Debug%20(Tests)/r90552%20(1104)/platform/mac/accessibility/html-slider-indicator-pretty-diff.html>.
Comment 15 Shinya Kawanaka 2011-07-11 02:15:40 PDT
(In reply to comment #14)
> Could this patch break <platform/mac/accessibility/html-slider-indicator.html>?
> 
> See <http://build.webkit.org/results/SnowLeopard%20Intel%20Debug%20(Tests)/r90552%20(1104)/platform/mac/accessibility/html-slider-indicator-pretty-diff.html>.

Sorry, I've made a bug entry to fix this problem.
https://bugs.webkit.org/show_bug.cgi?id=64256