New issue
Advanced search Search tips

Issue 819093 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression : Unnecessarily more than one radio button gets selected at the same time in chrome://settings page.

Reported by rp...@etouch.net, Mar 6 2018

Issue description

Version: 67.0.3362.0 (Official Build) Revisionbc0fc7083e5466f5227bc868c2543957672def9a-refs/heads/master@{#540777}(32/64-bit)
OS: Windows (7,8,8.1,10),Linux (14.04 LTS),Mac OS X(10.12.6,10.13.1,10.13.4)

What steps will reproduce the problem?
1. Launch chrome,navigate to chrome://settings/appearance
2. Now click on 'Show home button' and then click on 'Enter custom web address' text field
3. Now drag mouse cursor to select 'New tab page' text label and observe
 
Actual: Unnecessarily more than one radio button gets selected at the same time
Expected: Only one radio button should get selected at a time.

This is regression issue, broken in ‘M 66’ and will soon update other info :

Note : This issue is observed for all radio buttons on chrome://settings page.

 
Actual_video.mp4
144 KB View Download
Expected_video.mp4
126 KB View Download

Comment 1 by rp...@etouch.net, Mar 6 2018

Labels: hasbisect-per-revision Target-67 RegressedIn-66 FoundIn-67 FoundIn-66 Target-66
Owner: dpa...@chromium.org
Status: Assigned (was: Unconfirmed)
This is regression issue, broken in ‘M 66’ and below is the bisect info :
Good build: 66.0.3350.0 (Revision: 537343).
Bad build: 66.0.3352.0 (Revision: 538313).

You are probably looking for a change made after 537519 (known good), but no later than 537521 (first known bad).

CHANGELOG URL:

The script might not always return single CL as suspect as some perf builds might get missing due to failure.

https://chromium.googlesource.com/chromium/src/+log/2a6316cbf58a58b8f5e29758e645e43469aff312..04f68ff8420a40a405969c3251a119afb0697a96

Suspect : https://chromium.googlesource.com/chromium/src/+/04f68ff8420a40a405969c3251a119afb0697a96

From the CL above, assigning the issue to the concern owner 

@dpapad- Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Note : Issue is also seen on Dev build #66.0.3355.0
Labels: ReleaseBlock-Stable
marking as RBS, please change if required
Status: Started (was: Assigned)
Cc: dbeam@chromium.org
Found the reason this is happening. There are two things happening resulting in the bug.

1) IronSelectableBehavior only listens for 'tap' events, to determine that the selection changed, see [1].
2) When text is selected as shown in the screencast, a 'tap' event is not firing (Polymer suppreses it), but a native click event is firing.

The combinatino of 1 and 2 above, makes the button update itself to look as "selected", but the radio group does not un-select other buttons.


[1] https://cs.chromium.org/chromium/src/third_party/polymer/v1_0/components-chromium/iron-selector/iron-selectable-extracted.js?t&l=78
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe2c182624dfbf04145eadb5b69d64e6799365e6

commit fe2c182624dfbf04145eadb5b69d64e6799365e6
Author: dpapad <dpapad@chromium.org>
Date: Wed Mar 07 04:59:38 2018

Settings WebUI: Stop manually checking controlled-radio-button.

It seems unnecessary, since the parent paper-radio-group, already takes care of
checking/un-checking buttons, and in fact caused a bug, where multiple buttons
can appear checked at the same time (even though the paper-radio-group only
considers one of them as the actual selection).

Bug:  819093 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I66258840a743e777fd70ba927dad384fe7dff1fe
Reviewed-on: https://chromium-review.googlesource.com/952535
Reviewed-by: Dan Beam (no longer on Chrome) <dbeam@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541350}
[modify] https://crrev.com/fe2c182624dfbf04145eadb5b69d64e6799365e6/chrome/browser/resources/settings/controls/controlled_radio_button.js

Status: Fixed (was: Started)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-66; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-66 label, otherwise remove Merge-TBD label. Thanks.
Labels: -Merge-TBD Merge-Request-66

Comment 10 by rp...@etouch.net, Mar 8 2018

Labels: TE-Verified-M67 TE-Verified-67.0.3365.0
Update :
Rechecked the above issue on Windows (7,8,8.1,10),Linux (14.04 LTS) and Mac OS X(10.12.6,10.13.1,10.13.4)OS with latest Canary Chrome version :67.0.3365.0 and the issue is fixed.

Kindly refer attached screen cast for reference.
Actual_video.mp4
274 KB View Download
Project Member

Comment 11 by sheriffbot@chromium.org, Mar 8 2018

Labels: -Merge-Request-66 Merge-Approved-66 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M66. Please go ahead and merge the CL to branch 3359 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 12 by bugdroid1@chromium.org, Mar 8 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b48136d4f7d463a16aca77fecffa3362b7a921e1

commit b48136d4f7d463a16aca77fecffa3362b7a921e1
Author: dpapad <dpapad@chromium.org>
Date: Thu Mar 08 20:09:15 2018

[M66 merge] Settings WebUI: Stop manually checking controlled-radio-button.

It seems unnecessary, since the parent paper-radio-group, already takes care of
checking/un-checking buttons, and in fact caused a bug, where multiple buttons
can appear checked at the same time (even though the paper-radio-group only
considers one of them as the actual selection).

TBR=dpapad@chromium.org

(cherry picked from commit fe2c182624dfbf04145eadb5b69d64e6799365e6)

Bug:  819093 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I66258840a743e777fd70ba927dad384fe7dff1fe
Reviewed-on: https://chromium-review.googlesource.com/952535
Reviewed-by: Dan Beam (no longer on Chrome) <dbeam@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#541350}
Reviewed-on: https://chromium-review.googlesource.com/956284
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#112}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/b48136d4f7d463a16aca77fecffa3362b7a921e1/chrome/browser/resources/settings/controls/controlled_radio_button.js

Comment 13 by rp...@etouch.net, Mar 9 2018

Labels: TE-Verified-66.0.3359.22 TE-Verified-M66
Update :
Rechecked the above issue on Windows (7,8,8.1,10),Linux (14.04 LTS) and Mac OS X(10.12.6,10.13.1,10.13.4)OS with Dev build Chrome version :66.0.3359.22 and the issue is fixed.

Kindly refer attached screen-cast for reference.
Actual_video.mp4
241 KB View Download

Sign in to add a comment