Issue metadata
Sign in to add a comment
|
Resolution slider in M67 Beta updates resolution before the new increment is picked |
||||||||||||||||||||
Issue descriptionUser feedback report: https://listnr.corp.google.com/report/85454241829 URL: chrome://settings/display Product Version: 67.0.3396.49 beta User Agent: Mozilla/5.0 (X11; CrOS aarch64 10575.40.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.49 Safari/537.36 UI Language: en-US Description: Resolution UI is unusable if dragging the control 1. Start at max resolution 2. Hold down touchpad button, start dragging towards lower resolution 3. As resolution lowers, the UI is updated, and the current mouse position forces the mode to go to lower and lower resolutions until it reaches the lowest. Same problem when increasing resolutions. The only way this control works is by clicking on the desired resolution. Maybe we need a small delay before updating resolution, or wait for touchpad button release to update the resolution? Product Specific Data (whitelisted): CHROME VERSION: 67.0.3396.49 beta CHROMEOS_AUSERVER: <URL: 4> CHROMEOS_RELEASE_BOARD: elm-signed-mpkeys CHROMEOS_RELEASE_DESCRIPTION: 10575.40.0 (Official Build) beta-channel elm CHROMEOS_RELEASE_TRACK: beta-channel CHROMEOS_RELEASE_VERSION: 10575.40.0 ENTERPRISE_ENROLLED: Managed cpu: ARMv8 Processor rev 2 (v8l) expi: 3300005 3300108 3300134 3311879 3313322 hardware_class: ELM D4A-A2A-W5A-A2F routes: [ ip -4 rule list ] 0: from all lookup local 1: from all lookup main 32766: from all lookup main 32767: from all lookup default [ ip -4 route show table main ] default via <IPv4: 1> dev mlan0 metric 1 <IPv4: 15>/30 dev arcbr0 proto kernel scope link src <IPv4: 11> <IPv4: 30>/24 dev mlan0 proto kernel scope link src <IPv4: 1>06
,
May 22 2018
,
May 23 2018
,
May 29 2018
,
May 29 2018
,
May 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5730340bfabe9ffe98b1b72336a34ee506be4422 commit 5730340bfabe9ffe98b1b72336a34ee506be4422 Author: Malay Keshav <malaykeshav@chromium.org> Date: Tue May 29 19:44:34 2018 Fix immediate pref value change in display resolution settings We use a change observer to trigger updating the selected mode of a display. This is useful for the drop down menu we use to select the mode on external displays. However, if display zoom is disabled, we show a slider instead of a drop down. The slider triggers the same pref value change even if the user is still dragging it. This causes a usability regression on the settings resolution slider. To fix this, the patch adds a check which ensures we only listen to events fired due to releasing the slider when display zoom is disabled. Bug: 845712 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I9965bdf3121289ac29f38175cc9ad179cda41497 Component: Display settings, resolution slider Reviewed-on: https://chromium-review.googlesource.com/1069773 Commit-Queue: Malay Keshav <malaykeshav@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Cr-Commit-Position: refs/heads/master@{#562555} [modify] https://crrev.com/5730340bfabe9ffe98b1b72336a34ee506be4422/chrome/browser/resources/settings/device_page/display.html [modify] https://crrev.com/5730340bfabe9ffe98b1b72336a34ee506be4422/chrome/browser/resources/settings/device_page/display.js
,
May 29 2018
,
May 29 2018
This bug requires manual review: Request affecting a post-stable build Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 29 2018
Tested this manually on M67 build. The change merged without any conflict and is WAI.
,
May 30 2018
This is a fairly large change to merge as a P2. Really a RBS as a P2? Confirmed M67 regression?
,
May 30 2018
This is a P1, it's very difficult to operate the UI while the resolution is changing :)
,
May 30 2018
It is currently next to impossible to change the resolution with mouse or tap due to this regression. Users would have to use keyboard to change it.
,
Jun 4 2018
Please add details on testing status. Has the fix been tested? On more than one board? Any noted unintended side effects?
,
Jun 5 2018
Ping on testing status since this is tagged as a RBS
,
Jun 5 2018
The change is trivial, and does not depend on boards. It has been tested on cave and linux builds.
,
Jun 7 2018
Issue 848009 has been merged into this issue.
,
Jun 7 2018
Adding the conops Chrome OS hotlist for tracking.
,
Jun 13 2018
Per #12, would using the keyboard be an unreasonable workaround for the user? Again, large change this late into the workflow. Thanks,
,
Jun 13 2018
I don't think it's reasonable for users to understand how to use the keyboard to manipulate the slider. That's generally a power user or accessibility user feature.
,
Jun 13 2018
Thanks for the feedback. The plan is to merge/test in M68 beta, and pending those results merge into a follow-up M67 stable.
,
Jun 13 2018
This bug requires manual review: M68 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 13 2018
,
Jun 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/638621ef4970061838486e43573acbaea12a993f commit 638621ef4970061838486e43573acbaea12a993f Author: Malay Keshav <malaykeshav@chromium.org> Date: Wed Jun 13 19:41:05 2018 (merge) Fix immediate pref value change in display resolution settings We use a change observer to trigger updating the selected mode of a display. This is useful for the drop down menu we use to select the mode on external displays. However, if display zoom is disabled, we show a slider instead of a drop down. The slider triggers the same pref value change even if the user is still dragging it. This causes a usability regression on the settings resolution slider. To fix this, the patch adds a check which ensures we only listen to events fired due to releasing the slider when display zoom is disabled. Bug: 845712 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I9965bdf3121289ac29f38175cc9ad179cda41497 Component: Display settings, resolution slider Reviewed-on: https://chromium-review.googlesource.com/1069773 Commit-Queue: Malay Keshav <malaykeshav@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#562555}(cherry picked from commit 5730340bfabe9ffe98b1b72336a34ee506be4422) Reviewed-on: https://chromium-review.googlesource.com/1099695 Reviewed-by: Malay Keshav <malaykeshav@chromium.org> Cr-Commit-Position: refs/branch-heads/3440@{#340} Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733} [modify] https://crrev.com/638621ef4970061838486e43573acbaea12a993f/chrome/browser/resources/settings/device_page/display.html [modify] https://crrev.com/638621ef4970061838486e43573acbaea12a993f/chrome/browser/resources/settings/device_page/display.js
,
Jun 19 2018
Please update with testing results ASAP to have this considered for a M67 merge.
,
Jun 19 2018
Tested this on M67 ToB and its WAI for cave and eve. Did not see any critical side effects.
,
Jun 19 2018
Approving merge for M67 Chrome OS after review with the owner that this is low risk and fully tested.
,
Jun 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d74a5ec03cc1ca73cde1bb21bbbb6c65159bc349 commit d74a5ec03cc1ca73cde1bb21bbbb6c65159bc349 Author: Malay Keshav <malaykeshav@chromium.org> Date: Tue Jun 19 22:24:39 2018 (merge) Fix immediate pref value change in display resolution settings We use a change observer to trigger updating the selected mode of a display. This is useful for the drop down menu we use to select the mode on external displays. However, if display zoom is disabled, we show a slider instead of a drop down. The slider triggers the same pref value change even if the user is still dragging it. This causes a usability regression on the settings resolution slider. To fix this, the patch adds a check which ensures we only listen to events fired due to releasing the slider when display zoom is disabled. Bug: 845712 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I9965bdf3121289ac29f38175cc9ad179cda41497 Component: Display settings, resolution slider Reviewed-on: https://chromium-review.googlesource.com/1069773 Commit-Queue: Malay Keshav <malaykeshav@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#562555}(cherry picked from commit 5730340bfabe9ffe98b1b72336a34ee506be4422) Reviewed-on: https://chromium-review.googlesource.com/1106979 Reviewed-by: Malay Keshav <malaykeshav@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#780} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/d74a5ec03cc1ca73cde1bb21bbbb6c65159bc349/chrome/browser/resources/settings/device_page/display.html [modify] https://crrev.com/d74a5ec03cc1ca73cde1bb21bbbb6c65159bc349/chrome/browser/resources/settings/device_page/display.js |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by malaykeshav@chromium.org
, May 22 2018