New issue
Advanced search Search tips

Issue 888922 link

Starred by 2 users
Status: Fixed
Owner:
aee@chromium.org
Closed: Oct 26
Cc:
nyerramilli@chromium.org
rbasuvula@chromium.org
dpa...@chromium.org
rbpotter@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Regression: Focus traverses to disabled radio button on chrome://settings/syncSetup

Reported by vineetha...@etouch.net, Sep 25 
Chrome version : 71.0.3561.0 (Official Build) Revision 59edfd1d195efd57c937c950c1fd2a708a83f1f0-refs/branch-heads/3561@{#1}(32/64-bit) 
OS : Win(7,8,8.1,10), Mac(10.12.6, 10.13.1, 10.14, 10.13.6) and Linux(14.04 LTS) OS

Pre-condition: Sign in to chrome with an account having a sync passphrase associated with it.

Steps to reproduce:
1. Launch chrome and navigate to chrome://settings/syncSetup.
2. Enter valid passphrase in 'Passphrase' text field and press Enter key or click 'Submit' button. 
3. Now press tab to traverse focus throughout the page and observe focus under 'Encryption options' section.

Actual Result  : Tab focus traverses to 'Encryption options' > 'All data was encrypted..' radio button , even when it is disabled.
Expected Result: Tab focus should not travel to radio button when it is disabled.

This is a regression issue broken in ‘M-71’ and will soon update bisect info.
Good build: 71.0.3544.0(Revision: 589077)
Bad build : 71.0.3545.0(Revision: 589377)

Note: Press tab to get the focus on disabled radio button , now press down arrow key and observe radio button appears deselected.
ActualVideo.mp4
973 KB View Download
ExpectedVideo.mp4
930 KB View Download

Comment 1 by vineetha...@etouch.net, Sep 25

Labels: hasbisect-per-revision
Owner: tangltom@chromium.org
Status: Assigned (was: Unconfirmed)
You are probably looking for a change made after 589370 (known good), but no later than 589371 (first known bad).

CHANGE-LOG 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/b8fa1ab33246b17f5e4f1b1e9488460759d9d5f8..652922aab5efd6488608d7b23257c012457a61c1

Suspect: https://chromium.googlesource.com/chromium/src/+/652922aab5efd6488608d7b23257c012457a61c1

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

@tangltom - 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.

Thanks!

Comment 2 by tangltom@chromium.org, Sep 25

Labels: -Pri-1 Pri-2
Owner: dpa...@chromium.org
It's reproducible and I just investigated it a bit.
The issue is that one of the cr-radio-buttons has tabindex=0, even though it's disabled and aria-disabled. It's this one:

                <cr-radio-button name="encrypt-with-passphrase"
                    class="list-item" disabled$="[[syncPrefs.encryptAllData]]"
                    aria-labelledby="fullEncryptionBody">
                  <span id="fullEncryptionBody">
                    [[syncPrefs.fullEncryptionBody]]
                  </span>
                </cr-radio-button>

I looked into ui/webui/resources/cr_elements/cr_radio_button/cr_radio_button_behavior.js
 but I don't know how this can happen.
Can you take a look at this Demetrios?

Comment 3 by dpa...@chromium.org, Oct 1

Cc: dpa...@chromium.org
Owner: scottchen@chromium.org
Re-assigning to Scott, who authored cr-radio-button IIRC.

Comment 4 by scottchen@chromium.org, Oct 1

Owner: aee@chromium.org
It looks like iron-menu-behavior is setting it to 0 without caring whether or not things are disabled[1]. Assigning to Esmael since he's gonna start creating a cr-radio-group to replace paper-radio-group.

https://cs.chromium.org/chromium/src/third_party/polymer/v1_0/components-chromium/iron-menu-behavior/iron-menu-behavior-extracted.js?type=cs&sq=package:chromium&g=0&l=115

Comment 5 by dpa...@chromium.org, Oct 1

Cc: rbpotter@chromium.org
> Assigning to Esmael since he's gonna start creating a cr-radio-group to replace paper-radio-group.

Rephrasing a bit. Esmael, could you look into whether a cr-radio-group replacement for paper-radio-group would be reasonable? The hope is (similar to other such cases), that we can have a simpler element, that fixes all currently known issues about paper-radio-group, and stop depending on paper-radio-group.

Also rbpotter recently discovered another paper-radio-group problem (there is no bug filed for it yet). In short, changing the radio group selection programmatically does not correctly update the tab index.

Comment 6 by aee@chromium.org, Oct 2

Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 23 

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

commit 48bdf9871189ee1310a0871027ae58316cfb6c51
Author: Esmael El-Moslimany <aee@chromium.org>
Date: Tue Oct 23 19:41:41 2018

WebUI: create cr-radio-group, a replacement for paper-radio-group

Bug:  888922 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Icb6a31e501a5fbad60faa06cf8577c5212624e96
Reviewed-on: https://chromium-review.googlesource.com/c/1265090
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602059}
[modify] https://crrev.com/48bdf9871189ee1310a0871027ae58316cfb6c51/chrome/browser/resources/settings/controls/controlled_radio_button.js
[modify] https://crrev.com/48bdf9871189ee1310a0871027ae58316cfb6c51/chrome/test/data/webui/cr_elements/cr_elements_browsertest.js
[modify] https://crrev.com/48bdf9871189ee1310a0871027ae58316cfb6c51/chrome/test/data/webui/cr_elements/cr_radio_button_test.js
[add] https://crrev.com/48bdf9871189ee1310a0871027ae58316cfb6c51/chrome/test/data/webui/cr_elements/cr_radio_group_test.js
[modify] https://crrev.com/48bdf9871189ee1310a0871027ae58316cfb6c51/testing/buildbot/filters/webui_polymer2_browser_tests.filter
[modify] https://crrev.com/48bdf9871189ee1310a0871027ae58316cfb6c51/third_party/closure_compiler/externs/pending.js
[modify] https://crrev.com/48bdf9871189ee1310a0871027ae58316cfb6c51/ui/webui/resources/cr_elements/BUILD.gn
[modify] https://crrev.com/48bdf9871189ee1310a0871027ae58316cfb6c51/ui/webui/resources/cr_elements/cr_radio_button/cr_radio_button_behavior.js
[add] https://crrev.com/48bdf9871189ee1310a0871027ae58316cfb6c51/ui/webui/resources/cr_elements/cr_radio_group/BUILD.gn
[add] https://crrev.com/48bdf9871189ee1310a0871027ae58316cfb6c51/ui/webui/resources/cr_elements/cr_radio_group/cr_radio_group.html
[add] https://crrev.com/48bdf9871189ee1310a0871027ae58316cfb6c51/ui/webui/resources/cr_elements/cr_radio_group/cr_radio_group.js
[modify] https://crrev.com/48bdf9871189ee1310a0871027ae58316cfb6c51/ui/webui/resources/cr_elements_resources.grdp
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 23 

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

commit 812feefb9de26bb9df2d42beb62eeae43c6c64e1
Author: Esmael El-Moslimany <aee@chromium.org>
Date: Tue Oct 23 22:58:56 2018

WebUI: replace paper-radio-group with cr-radio-group

Bug:  888922 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I703c74ed1e849bf9fb3814976163dd2a6b6a3a54
Reviewed-on: https://chromium-review.googlesource.com/c/1263561
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602152}
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/chromeos/emulator/audio_settings.html
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/chromeos/emulator/battery_settings.html
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/chromeos/emulator/bluetooth_settings.html
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/chromeos/login/enrollment_license_card.html
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/chromeos/login/sync_consent.css
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/chromeos/login/sync_consent.html
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/settings/about_page/channel_switcher_dialog.html
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/settings/about_page/channel_switcher_dialog.js
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/settings/appearance_page/home_url_input.js
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/settings/controls/settings_radio_group.html
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/settings/controls/settings_radio_group.js
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/settings/multidevice_page/multidevice_smartlock_subpage.html
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/settings/multidevice_page/multidevice_smartlock_subpage.js
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/settings/people_page/lock_screen.html
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/settings/people_page/sync_page.html
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/settings/people_page/sync_page.js
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/settings/settings_shared_css.html
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/settings/settings_vars_css.html
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/signin/dice_sync_confirmation/sync_confirmation_app.html
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/signin/dice_sync_confirmation/sync_confirmation_app.js
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/signin/signin_email_confirmation/signin_email_confirmation.html
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/browser/resources/signin/signin_email_confirmation/signin_email_confirmation.js
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/test/data/webui/cr_elements/cr_radio_button_test.js
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/test/data/webui/cr_elements/cr_radio_group_test.js
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/test/data/webui/settings/about_page_tests.js
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/test/data/webui/settings/device_page_tests.js
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/test/data/webui/settings/on_startup_page_tests.js
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/chrome/test/data/webui/settings/people_page_sync_page_test.js
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/ui/webui/resources/cr_components/chromeos/network/network_nameservers.html
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/ui/webui/resources/cr_components/chromeos/network/network_nameservers.js
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/ui/webui/resources/cr_elements/cr_radio_button/cr_radio_button_behavior.js
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/ui/webui/resources/cr_elements/cr_radio_button/cr_radio_button_style_css.html
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/ui/webui/resources/cr_elements/cr_radio_group/cr_radio_group.js
[modify] https://crrev.com/812feefb9de26bb9df2d42beb62eeae43c6c64e1/ui/webui/resources/cr_elements/shared_style_css.html
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 24 

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

commit 13a852e7113473feb4d39f6159a3edbd06ddd2db
Author: Esmael El-Moslimany <aee@chromium.org>
Date: Wed Oct 24 02:54:05 2018

Print Preview WebUI: replace paper-radio-group with cr-radio-group

Bug:  888922 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I7ab9988ec40f715979cf94ec2d8fd94e87723d4f
Reviewed-on: https://chromium-review.googlesource.com/c/1277868
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602232}
[modify] https://crrev.com/13a852e7113473feb4d39f6159a3edbd06ddd2db/chrome/browser/resources/print_preview/new/pages_settings.html
[modify] https://crrev.com/13a852e7113473feb4d39f6159a3edbd06ddd2db/chrome/browser/resources/print_preview/new/pages_settings.js
[modify] https://crrev.com/13a852e7113473feb4d39f6159a3edbd06ddd2db/chrome/test/data/webui/print_preview/pages_settings_test.js
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 24 

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

commit 309585b94e917f385234d215a8091fb61c2cf0bf
Author: Esmael El-Moslimany <aee@chromium.org>
Date: Wed Oct 24 03:53:14 2018

WebUI: removing paper-radio-group

This is a follow-up CL to creating an alternative radio group called
cr-radio-group and replacing all usages of paper-radio-group with
cr-radio-group.

Bug:  888922 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I6637190f8eb63409c16ff70f6a7bc65872442780
Reviewed-on: https://chromium-review.googlesource.com/c/1277526
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602249}
[modify] https://crrev.com/309585b94e917f385234d215a8091fb61c2cf0bf/third_party/polymer/v1_0/bower.json
[modify] https://crrev.com/309585b94e917f385234d215a8091fb61c2cf0bf/third_party/polymer/v1_0/chromium.patch
[delete] https://crrev.com/066cb5ad04248407c0107fbbbf8f910ea7a24255/third_party/polymer/v1_0/components-chromium/paper-radio-group/BUILD.gn
[delete] https://crrev.com/066cb5ad04248407c0107fbbbf8f910ea7a24255/third_party/polymer/v1_0/components-chromium/paper-radio-group/bower.json
[delete] https://crrev.com/066cb5ad04248407c0107fbbbf8f910ea7a24255/third_party/polymer/v1_0/components-chromium/paper-radio-group/paper-radio-group-extracted.js
[delete] https://crrev.com/066cb5ad04248407c0107fbbbf8f910ea7a24255/third_party/polymer/v1_0/components-chromium/paper-radio-group/paper-radio-group.html
[modify] https://crrev.com/309585b94e917f385234d215a8091fb61c2cf0bf/third_party/polymer/v1_0/components_summary.txt
[modify] https://crrev.com/309585b94e917f385234d215a8091fb61c2cf0bf/third_party/polymer/v1_0/rsync_exclude.txt
[modify] https://crrev.com/309585b94e917f385234d215a8091fb61c2cf0bf/ui/webui/resources/polymer_resources.grdp
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 25 

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

commit 2673aeb9ff9b019253b3393c1fb6ffe48b3d4a8f
Author: Esmael El-Moslimany <aee@chromium.org>
Date: Thu Oct 25 22:36:12 2018

Settings WebUI: removing duplicate functionality from multidevice-radio-button

This functionality already exists in CrRadioButtonBehavior.

https://cs.chromium.org/chromium/src/ui/webui/resources/cr_elements/cr_radio_button/cr_radio_button_behavior.js?l=19

Bug:  888922 
Change-Id: Ie143f7605d858ea368be9ff7c3f876e05180739d
Reviewed-on: https://chromium-review.googlesource.com/c/1298294
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602902}
[modify] https://crrev.com/2673aeb9ff9b019253b3393c1fb6ffe48b3d4a8f/chrome/browser/resources/settings/multidevice_page/multidevice_radio_button.js
[modify] https://crrev.com/2673aeb9ff9b019253b3393c1fb6ffe48b3d4a8f/chrome/browser/resources/settings/multidevice_page/multidevice_smartlock_subpage.html

Comment 12 by vineetha...@etouch.net, Oct 26

Labels: TE-Verified-M72 TE-Verified-72.0.3592.0
Update :
Rechecked the above issue on  Win(7,8,8.1,10), Mac(10.12.6, 10.13.1, 10.14, 10.13.6) and Linux(14.04 LTS) OS with latest Canary version #72.0.3592.0 and the issue is fixed.

Kindly refer the attached screen cast.
CanaryBehaviour.mp4
1012 KB View Download

Comment 13 by aee@chromium.org, Oct 26

Status: Fixed (was: Started)
►

Sign in to add a comment