Make the same key (e.g. Left) do the same thing for horiz/vert range

Prior to this CL, a horizontal range slider with ltr text direction
would increase in value if Right Arrow is pressed, and decrease for
Left Arrow. However, if the range were vertical (with
-webkit-appearance: slider-vertical), those directions were reversed:
Right Arrow decreases the value, while Left Arrow increases it.
Similarly, Home and End keys had reversed operation for vertical
ranges.

With this CL, the same keys produce the same (semantic) result.

Fixed: 1162640
Change-Id: Ia2251ffe9e2056f1c20d25130b8a0d2e3d1e79ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2623117Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Auto-Submit: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842820}

third_party/blink/renderer/core/html/forms/range_input_type.cc

@@ -194,12 +194,8 @@ void RangeInputType::HandleKeydownEvent(KeyboardEvent& event) {
       std::max((step_range.Maximum() - step_range.Minimum()) / 10, step);
 
   TextDirection dir = TextDirection::kLtr;
-  bool is_vertical = false;
   if (GetElement().GetLayoutObject()) {
     dir = ComputedTextDirection();
-    ControlPart part =
-        GetElement().GetLayoutObject()->Style()->EffectiveAppearance();
-    is_vertical = part == kSliderVerticalPart;
   }
 
   Decimal new_value;
@@ -208,19 +204,17 @@ void RangeInputType::HandleKeydownEvent(KeyboardEvent& event) {
   } else if (key == "ArrowDown") {
     new_value = current - step;
   } else if (key == "ArrowLeft") {
-    new_value = (is_vertical || dir == TextDirection::kRtl) ? current + step
-                                                            : current - step;
+    new_value = dir == TextDirection::kRtl ? current + step : current - step;
   } else if (key == "ArrowRight") {
-    new_value = (is_vertical || dir == TextDirection::kRtl) ? current - step
-                                                            : current + step;
+    new_value = dir == TextDirection::kRtl ? current - step : current + step;
   } else if (key == "PageUp") {
     new_value = current + big_step;
   } else if (key == "PageDown") {
     new_value = current - big_step;
   } else if (key == "Home") {
-    new_value = is_vertical ? step_range.Maximum() : step_range.Minimum();
+    new_value = step_range.Minimum();
   } else if (key == "End") {
-    new_value = is_vertical ? step_range.Minimum() : step_range.Maximum();
+    new_value = step_range.Maximum();
   } else {
     return;  // Did not match any key binding.
   }

third_party/blink/web_tests/fast/forms/range/range-keyoperation-expected.txt

@@ -34,8 +34,14 @@ PASS input.value is "100"
 PASS changeEventCounter is lastChangeEventCounter
 
 
-RTL
+Tests for a horizontal range with RTL
+Press the up arrow key:
 PASS input.value is "51"
+Press the down arrow key:
+PASS input.value is "50"
+Press the left arrow key:
+PASS input.value is "51"
+Press the right arrow key:
 PASS input.value is "50"
 
 
@@ -45,7 +51,7 @@ PASS input.value is "51"
 Press the down arrow key:
 PASS input.value is "50"
 Press the left arrow key:
-PASS input.value is "51"
+PASS input.value is "49"
 Press the right arrow key:
 PASS input.value is "50"
 Press the PageUp key:
@@ -53,9 +59,28 @@ PASS input.value is "60"
 Press the PageDown key:
 PASS input.value is "50"
 Press the Home key:
-PASS input.value is "100"
+PASS input.value is "0"
 Press the End key:
+PASS input.value is "100"
+
+
+Tests for a vertical range with rtl
+Press the up arrow key:
+PASS input.value is "51"
+Press the down arrow key:
+PASS input.value is "50"
+Press the left arrow key:
+PASS input.value is "51"
+Press the right arrow key:
+PASS input.value is "50"
+Press the PageUp key:
+PASS input.value is "60"
+Press the PageDown key:
+PASS input.value is "50"
+Press the Home key:
 PASS input.value is "0"
+Press the End key:
+PASS input.value is "100"
 
 
 step=any cases
@@ -72,10 +97,10 @@ Press the pagedown key:
 PASS input.value is "100"
 PASS changeEventCounter is lastChangeEventCounter + 1
 Press the home key:
-PASS input.value is "200"
+PASS input.value is "0"
 PASS changeEventCounter is lastChangeEventCounter + 1
 Press the end key:
-PASS input.value is "0"
+PASS input.value is "200"
 PASS changeEventCounter is lastChangeEventCounter + 1
 Edge cases
 PASS input.value is "200"
@@ -99,9 +124,9 @@ PASS input.value is "20"
 small range
 PASS input.value is "9"
 PASS input.value is "6"
-PASS input.value is "0"
 PASS input.value is "9"
 PASS input.value is "0"
+PASS input.value is "9"
 
 
 Disabled and read-only
@@ -117,3 +142,4 @@ PASS successfullyParsed is true
 
 TEST COMPLETE
 
+      

third_party/blink/web_tests/fast/forms/range/range-keyoperation.html

@@ -6,6 +6,15 @@
 <body>
 <p id="description">Test for keyboard operations of &lt;input type=range></p>
 <div id="console"></div>
+
+<input type=range id=horiz-range min=0 max=100 value=50 onchange='handleChange()'>
+<input type=range id=horiz-range-rtl dir=rtl min=0 max=100 value=50 onchange='handleChange()'>
+<input type=range id=vert-range min=0 max=100 value=50 onchange='handleChange()' style='-webkit-appearance:slider-vertical;'>
+<input type=range id=vert-range-rtl dir=rtl min=0 max=100 value=50 onchange='handleChange()' style='-webkit-appearance:slider-vertical;'>
+<input type=range id=step-any min=0 max=200 value=100 step='any' onchange='handleChange()' style='-webkit-appearance:slider-vertical;'>
+<input type=range id=small-range min=0 max=10 value=6 step=3 onchange='handleChange()' style='-webkit-appearance:slider-vertical;'>
+<input type=range id=disabled disabled min=0 max=100 value=1 step=1 onchange='handleChange()' style='-webkit-appearance:slider-vertical;'>
+
 <script>
 
 function sendKey(element, keyName) {
@@ -17,15 +26,10 @@ function handleChange() {
   changeEventCounter++;
 }
 
-var parent = document.createElement('div');
-document.body.appendChild(parent);
-parent.innerHTML = '<input type=range id=range min=0 max=100 value=50>';
-
-var input = document.getElementById('range');
-input.onchange = handleChange;
+debug('Tests for a horizontal range');
+var input = document.getElementById('horiz-range');
 input.focus();
 
-debug('Tests for a horizontal range');
 debug('Press the up arrow key:');
 var lastChangeEventCounter = changeEventCounter;
 sendKey(input, 'ArrowUp');
@@ -92,23 +96,36 @@ sendKey(input, 'End');
 shouldBe('input.value', '"100"');
 shouldBe('changeEventCounter', 'lastChangeEventCounter');
 
-input.dir = 'rtl';
-input.value = '50';
-input.offsetLeft;
+
+
 debug('');
-debug('RTL');
+debug('Tests for a horizontal range with RTL');
+var input = document.getElementById('horiz-range-rtl');
+input.focus();
+
+debug('Press the up arrow key:');
+sendKey(input, 'ArrowUp');
+shouldBe('input.value', '"51"');
+
+debug('Press the down arrow key:');
+sendKey(input, 'ArrowDown');
+shouldBe('input.value', '"50"');
+
+debug('Press the left arrow key:');
 sendKey(input, 'ArrowLeft');
-shouldBeEqualToString('input.value', '51');
+shouldBe('input.value', '"51"');
+
+debug('Press the right arrow key:');
 sendKey(input, 'ArrowRight');
-shouldBeEqualToString('input.value', '50');
-input.dir = 'ltr';
+shouldBe('input.value', '"50"');
+
 
-debug('');
-input.setAttribute('style', '-webkit-appearance:slider-vertical; height: 40px;');
-input.offsetLeft; // force layout
 
-input.valueAsNumber = 50;
+debug('');
 debug('Tests for a vertical range');
+var input = document.getElementById('vert-range');
+input.focus();
+
 debug('Press the up arrow key:');
 sendKey(input, 'ArrowUp');
 shouldBe('input.value', '"51"');
@@ -119,7 +136,7 @@ shouldBe('input.value', '"50"');
 
 debug('Press the left arrow key:');
 sendKey(input, 'ArrowLeft');
-shouldBe('input.value', '"51"');
+shouldBe('input.value', '"49"');
 
 debug('Press the right arrow key:');
 sendKey(input, 'ArrowRight');
@@ -135,18 +152,57 @@ shouldBe('input.value', '"50"');
 
 debug('Press the Home key:');
 sendKey(input, 'Home');
-shouldBe('input.value', '"100"');
+shouldBe('input.value', '"0"');
 
 debug('Press the End key:');
 sendKey(input, 'End');
+shouldBe('input.value', '"100"');
+
+
+
+debug('');
+debug('Tests for a vertical range with rtl');
+var input = document.getElementById('vert-range-rtl');
+input.focus();
+
+debug('Press the up arrow key:');
+sendKey(input, 'ArrowUp');
+shouldBe('input.value', '"51"');
+
+debug('Press the down arrow key:');
+sendKey(input, 'ArrowDown');
+shouldBe('input.value', '"50"');
+
+debug('Press the left arrow key:');
+sendKey(input, 'ArrowLeft');
+shouldBe('input.value', '"51"');
+
+debug('Press the right arrow key:');
+sendKey(input, 'ArrowRight');
+shouldBe('input.value', '"50"');
+
+debug('Press the PageUp key:');
+sendKey(input, 'PageUp');
+shouldBe('input.value', '"60"');
+
+debug('Press the PageDown key:');
+sendKey(input, 'PageDown');
+shouldBe('input.value', '"50"');
+
+debug('Press the Home key:');
+sendKey(input, 'Home');
 shouldBe('input.value', '"0"');
 
+debug('Press the End key:');
+sendKey(input, 'End');
+shouldBe('input.value', '"100"');
+
+
+
 debug('');
 debug('step=any cases');
-input.step = 'any';
-input.min = '0';
-input.max = '200';
-input.valueAsNumber = 100;
+var input = document.getElementById('step-any');
+input.focus();
 
 debug('Press the up arrow key:');
 lastChangeEventCounter = changeEventCounter;
@@ -175,13 +231,13 @@ shouldBe('changeEventCounter', 'lastChangeEventCounter + 1');
 debug('Press the home key:');
 lastChangeEventCounter = changeEventCounter;
 sendKey(input, 'Home');
-shouldBe('input.value', '"200"');
+shouldBe('input.value', '"0"');
 shouldBe('changeEventCounter', 'lastChangeEventCounter + 1');
 
 debug('Press the end key:');
 lastChangeEventCounter = changeEventCounter;
 sendKey(input, 'End');
-shouldBe('input.value', '"0"');
+shouldBe('input.value', '"200"');
 shouldBe('changeEventCounter', 'lastChangeEventCounter + 1');
 
 debug('Edge cases');
@@ -225,12 +281,12 @@ shouldBe('changeEventCounter', 'lastChangeEventCounter');
 sendKey(input, 'PageUp');
 shouldBe('input.value', '"20"');
 
+
+
 debug('');
 debug('small range');
-input.min = '0';
-input.max = '10';
-input.step = '3';
-input.valueAsNumber = 6;
+var input = document.getElementById('small-range');
+input.focus();
 
 sendKey(input, 'PageUp');
 shouldBe('input.value', '"9"');
@@ -239,23 +295,21 @@ sendKey(input, 'PageDown');
 shouldBe('input.value', '"6"');
 
 sendKey(input, 'End');
-shouldBe('input.value', '"0"');
+shouldBe('input.value', '"9"');
 
 sendKey(input, 'Home');
-shouldBe('input.value', '"9"');
+shouldBe('input.value', '"0"');
 
 sendKey(input, 'End');
-shouldBe('input.value', '"0"');
+shouldBe('input.value', '"9"');
+
 
 
 debug('');
 debug('Disabled and read-only');
-input.min = '0';
-input.max = '100';
-input.step = '1';
-input.value = '1';
+var input = document.getElementById('disabled');
+input.focus();
 
-input.disabled = true;
 sendKey(input, 'ArrowUp');
 shouldBe('input.value', '"1"');
 sendKey(input, 'Home');