Color picker can have NaN values for HSL and Hex formats at zoom 150%

This change fixes a bug where the HSL and Hex values for the selected color could be NaN if the color well's selection ring was the bottom of the color well and the page zoom was set to 150%. At certain zoom levels like 150%, the height of the color well is not a round number. However, the getImageData API, which is used to retrieve the underlying pixel color data for the color well, only works with integer values and will truncate decimal values. As such, if the color well's selection ring is placed at the bottom of the color well, a valid pixel data point for the ring's position cannot be found in the image data array for the color well. When such a situation occurs, the fix is to use the last color in the color well's image data array. This color will be the same as the color seen at the bottom of the color well (black), so the user will see what they expect. Bug: 1015238 Change-Id: I1b74a4870fa455d197c525dca39dc88e7c6f75f9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1866060Reviewed-by: Kent Tamura <tkent@chromium.org> Commit-Queue: Sanket Joshi <sajos@microsoft.com> Cr-Commit-Position: refs/heads/master@{#706758}

Color picker can have NaN values for HSL and Hex formats at zoom 150%
This change fixes a bug where the HSL and Hex values for the selected color could be NaN if the color well's selection ring was the bottom of the color well and the page zoom was set to 150%. At certain zoom levels like 150%, the height of the color well is not a round number. However, the getImageData API, which is used to retrieve the underlying pixel color data for the color well, only works with integer values and will truncate decimal values. As such, if the color well's selection ring is placed at the bottom of the color well, a valid pixel data point for the ring's position cannot be found in the image data array for the color well. When such a situation occurs, the fix is to use the last color in the color well's image data array. This color will be the same as the color seen at the bottom of the color well (black), so the user will see what they expect. Bug: 1015238 Change-Id: I1b74a4870fa455d197c525dca39dc88e7c6f75f9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1866060Reviewed-by: Kent Tamura <tkent@chromium.org> Commit-Queue: Sanket Joshi <sajos@microsoft.com> Cr-Commit-Position: refs/heads/master@{#706758}
ec74d092 · Sanket Joshi · Commit Bot · 98dde10a · ec74d092 · ec74d092
Commit ec74d092 authored Oct 17, 2019 by Sanket Joshi Committed by Commit Bot Oct 17, 2019
2 changed files
--- a/third_party/blink/renderer/core/html/forms/resources/color_picker.js
+++ b/third_party/blink/renderer/core/html/forms/resources/color_picker.js
@@ -874,7 +874,17 @@ class ColorPalette extends HTMLCanvasElement {
   * @param {number} y
   */
  hslImageDataAtPoint_(x, y) {
-    const offset = Math.round(y * this.width + x) * 3;
+    let offset = Math.round(y * this.width + x) * 3;
+    // It is possible that the computed offset is larger than the hslImageData
+    // array's length. This can happen at certain zoom levels (ex. 150%), where
+    // the height of the color well is not a round number. The getImageData API
+    // only works with integer values and will truncate decimal values. As
+    // such, if the color well's selection ring is placed at the bottom of the
+    // color well at such a zoom level, a valid data point for the ring's
+    // position will not be found in the hslImageData array. When this happens,
+    // we just report the color at the end of the hslImageData array. This will
+    // be the same color that is seen at the bottom of the color well (black).
+    offset = Math.min(offset, this.hslImageData.length - 3);
    return this.hslImageData.slice(offset, offset + 3);
  }


--- a/third_party/blink/web_tests/fast/forms/controls-new-ui/color/color-picker-zoom150-bottom-edge-no-nan.html
+++ b/third_party/blink/web_tests/fast/forms/controls-new-ui/color/color-picker-zoom150-bottom-edge-no-nan.html
+<!DOCTYPE html>
+<html>
+<head>
+<script src='../../../../resources/testharness.js'></script>
+<script src='../../../../resources/testharnessreport.js'></script>
+<script src='../../../forms/resources/picker-common.js'></script>
+</head>
+<body style='zoom: 1.5;'>
+<input type='color' id='color' value='#80d9ff'>
+<script>
+'use strict';
+
+let t = async_test('Color picker: Color values at the bottom edge of the color picker at zoom 150% should be valid numbers.');
+t.step(() => {
+  let colorControl = document.getElementById('color');
+  openPicker(colorControl, t.step_func_done(() => {
+    popupWindow.focus();
+    const popupDocument = popupWindow.document;
+    const hexValueContainer = popupDocument.getElementById('hexValueContainer');
+    assert_equals(hexValueContainer.value, '#80d9ff');
+    eventSender.keyDown('Tab');
+    const colorWellSelectionRing = popupDocument.querySelector('color-well > color-selection-ring');
+    assert_equals(popupDocument.activeElement, colorWellSelectionRing);
+    for (let i = 0; i < 10; i++) {
+      eventSender.keyDown('ArrowDown', ['ctrlKey']);
+    }
+    assert_equals(hexValueContainer.value, '#000000', 'Selected color value should be \'#000000\'');
+  }), t.step_func_done(() => {
+    assert_false(internals.runtimeFlags.formControlsRefreshEnabled, "Popup should only not open when the formControlsRefresh flag is disabled.");
+  }));
+});
+</script>
+</body>
+</html>
\ No newline at end of file