MD Settings: Restore focus after closing dialog |
|||||||||||||||||
Issue description1. Tab to a button that opens a dialog, like "Clear browsing data" 2. Press Enter to open the dialog 3. Press Esc (or alt-left) to close the dialog Expected: Focus is on the button that opened the dialog Actual: Focus is nowhere
,
Dec 6 2016
@dpapad is this <dialog> issue something you could look into?
,
Dec 6 2016
Yes. I've already done this for cr-action-menu dialogs at https://codereview.chromium.org/2464873003. We could do it for cr-dialog too. Basically all callers would need to pass an "anchorElement" parameter when requesting the dialog to be shown. Then when the dialog closes, it restores focus to that element.
,
Jan 7 2017
,
Feb 3 2017
Issue 686622 has been merged into this issue.
,
Feb 22 2017
Issue 686312 has been merged into this issue.
,
Feb 22 2017
Updating priority based on merged issue. This is needed for basic a11y.
,
Feb 22 2017
@lpalmaro where should focus go when the element shows a deletion confirmation dialog and is then deleted?
,
Feb 23 2017
I don't know if there's a perfect answer, but here's a thought: where would you expect focus to go if you deleted that element (e.g. by pressing Backspace) and no confirmation dialog was required? If it was in a list of elements, focus would probably go to an adjacent / sibling element, so when the dialog closes I'd expect something similar to happen.
,
Feb 27 2017
The vanilla HTML before is to keep the imaginary focus "caret" in the same place, but not focus anything. Tab or Shift+Tab will cause the next or previous element to be focused. https://jsfiddle.net/tmq3h656/ This actually works for a native dialog too: https://jsfiddle.net/w3kgnypz/ with basically the same repro steps as the issue description. I don't know if this "imaginary focus caret" is something we can manipulate in JS or a native feature.
,
Feb 27 2017
The vanilla HTML behavior*, not "before"
,
Mar 1 2017
Issue 630197 has been merged into this issue.
,
Mar 8 2017
hcarmona@: can you go through and play with our dialogs and see how well they currently function? note: as I mentioned to stevenjb@ earlier (and as michaelpg@ mentioned here), if a dialog is hidden, the "invisible focus caret" is left where the dialog was. Meaning, if you pressing Tab or Shift+Tab after closing a dialog, the next thing in the "sequential tab focus order" gets focus. this is from where the <dialog> is, which might not always be close to the *control that shows the dialog*. This is probably why folks are having unexpected results in some cases. Can we see if it's possible to move <dialog>s right next to their triggers or somewhere that at least makes some logical sense? Are there many dialogs with multiple triggers? Does moving their positioning have negative side effects for the code or conflict with any styles (i.e. does the dialog get clipped by parents with overflow or position)?
,
Mar 8 2017
,
Mar 14 2017
,
Mar 15 2017
Just a quick update on this issue. These are the places I'm looking at for cr-dialog. https://cs.chromium.org/search/?q=is%3D%5C%22cr-dialog%5C%22+file:html$+file:settings&m=100&type=cs
,
Mar 17 2017
,
Mar 17 2017
,
Mar 20 2017
+falken, +dmazzoni: This is related to issue 298078, but with a slight twist. When a dialog is closed AND then removed from the DOM, the focus behaves differently in the following cases: 1. <dialog> within a shadowRoot AND contains a focusable element: Focus is reset to the start of the document. 2 <dialog> within shadowRoot AND DOES NOT contain any focusable element, OR <dialog> not a child of a shadowRoot: Focus is preserved. hcarmona@ came up with a nice minimal repro at http://jsbin.com/tuwiyaw/19/edit?js,output. The difference in behavior between 1 and 2 seems like a bug, and not like expected speced behavior. Can you verify and potentially help identify a fix? This is affecting our MD Settings page, where we make heavy use of <dialog>s.
,
Mar 20 2017
,
Mar 21 2017
+kochi: see comment 19 about dialog and shadow dom integration
,
Mar 21 2017
,
Mar 27 2017
hcarmona@: what's the status of this bug? please keep updating it until fixed.
,
Mar 28 2017
,
Mar 29 2017
FYI, a potentially cheap (because it is centralized) workaround until the underlying issue is fixed is to call this.blur() at line https://cs.chromium.org/chromium/src/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js?l=54, right before we call the superclass close() method (and do he same when 'Esc' is pressed).
,
Mar 29 2017
Refined my suggestion at comment#25 a bit, see proof-of-concept at https://codereview.chromium.org/2779903003 (have not hooked up the 'Esc' key to the workaround).
,
Mar 29 2017
,
Mar 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/653ec948f369a89d8f2f6a7e6afbb323d9fb82ee commit 653ec948f369a89d8f2f6a7e6afbb323d9fb82ee Author: dpapad <dpapad@chromium.org> Date: Fri Mar 31 18:13:11 2017 MD Settings: Restore focus after closing dialogs, for passwords page. BUG= 668313 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2789783002 Cr-Commit-Position: refs/heads/master@{#461172} [modify] https://crrev.com/653ec948f369a89d8f2f6a7e6afbb323d9fb82ee/chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html [modify] https://crrev.com/653ec948f369a89d8f2f6a7e6afbb323d9fb82ee/chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.js [modify] https://crrev.com/653ec948f369a89d8f2f6a7e6afbb323d9fb82ee/chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.js
,
Mar 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/31a445667f4fddb5c78a464a4de52f000777a74a commit 31a445667f4fddb5c78a464a4de52f000777a74a Author: dpapad <dpapad@chromium.org> Date: Fri Mar 31 20:38:06 2017 MD Settings: Restore focus after closing dialogs, for startup page. BUG= 668313 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2782393004 Cr-Commit-Position: refs/heads/master@{#461212} [modify] https://crrev.com/31a445667f4fddb5c78a464a4de52f000777a74a/chrome/browser/resources/settings/on_startup_page/startup_url_entry.js [modify] https://crrev.com/31a445667f4fddb5c78a464a4de52f000777a74a/chrome/browser/resources/settings/on_startup_page/startup_urls_page.js [modify] https://crrev.com/31a445667f4fddb5c78a464a4de52f000777a74a/chrome/test/data/webui/settings/startup_urls_page_test.js
,
Apr 3 2017
,
Apr 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/04741e074fcddd1a3b830a85d054034e0b2a34cb commit 04741e074fcddd1a3b830a85d054034e0b2a34cb Author: dpapad <dpapad@chromium.org> Date: Tue Apr 04 01:20:31 2017 MD Settings: Restore focus after closing dialogs, for content settings. BUG= 668313 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2791053002 Cr-Commit-Position: refs/heads/master@{#461597} [modify] https://crrev.com/04741e074fcddd1a3b830a85d054034e0b2a34cb/chrome/browser/resources/settings/site_settings/site_data.html [modify] https://crrev.com/04741e074fcddd1a3b830a85d054034e0b2a34cb/chrome/browser/resources/settings/site_settings/site_data.js [modify] https://crrev.com/04741e074fcddd1a3b830a85d054034e0b2a34cb/chrome/browser/resources/settings/site_settings/site_list.html [modify] https://crrev.com/04741e074fcddd1a3b830a85d054034e0b2a34cb/chrome/browser/resources/settings/site_settings/site_list.js
,
Apr 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/79d1fe73c50b7a5f0ef25daed2a637710cb7e440 commit 79d1fe73c50b7a5f0ef25daed2a637710cb7e440 Author: dpapad <dpapad@chromium.org> Date: Tue Apr 04 02:46:07 2017 MD Settings: Restore focus after closing various dialogs. This CL addresses dialogs in - people-page - privacy-page (but not site settings) - reset-page - search-engines-page - languages-page BUG= 668313 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2793463002 Cr-Commit-Position: refs/heads/master@{#461616} [modify] https://crrev.com/79d1fe73c50b7a5f0ef25daed2a637710cb7e440/chrome/browser/resources/settings/languages_page/languages_page.js [modify] https://crrev.com/79d1fe73c50b7a5f0ef25daed2a637710cb7e440/chrome/browser/resources/settings/people_page/people_page.html [modify] https://crrev.com/79d1fe73c50b7a5f0ef25daed2a637710cb7e440/chrome/browser/resources/settings/people_page/people_page.js [modify] https://crrev.com/79d1fe73c50b7a5f0ef25daed2a637710cb7e440/chrome/browser/resources/settings/privacy_page/privacy_page.html [modify] https://crrev.com/79d1fe73c50b7a5f0ef25daed2a637710cb7e440/chrome/browser/resources/settings/privacy_page/privacy_page.js [modify] https://crrev.com/79d1fe73c50b7a5f0ef25daed2a637710cb7e440/chrome/browser/resources/settings/reset_page/reset_page.html [modify] https://crrev.com/79d1fe73c50b7a5f0ef25daed2a637710cb7e440/chrome/browser/resources/settings/reset_page/reset_page.js [modify] https://crrev.com/79d1fe73c50b7a5f0ef25daed2a637710cb7e440/chrome/browser/resources/settings/search_engines_page/search_engine_entry.js [modify] https://crrev.com/79d1fe73c50b7a5f0ef25daed2a637710cb7e440/chrome/browser/resources/settings/search_engines_page/search_engines_page.js
,
Apr 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d54ece4d6be085d618f0210036a7cb373e029c49 commit d54ece4d6be085d618f0210036a7cb373e029c49 Author: dpapad <dpapad@chromium.org> Date: Wed Apr 05 18:50:30 2017 MD Settings: Restore focus after closing dialogs, for certificate page. BUG= 668313 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2792663002 Cr-Commit-Position: refs/heads/master@{#462161} [modify] https://crrev.com/d54ece4d6be085d618f0210036a7cb373e029c49/chrome/browser/resources/settings/certificate_manager_page/ca_trust_edit_dialog.js [modify] https://crrev.com/d54ece4d6be085d618f0210036a7cb373e029c49/chrome/browser/resources/settings/certificate_manager_page/certificate_delete_confirmation_dialog.js [modify] https://crrev.com/d54ece4d6be085d618f0210036a7cb373e029c49/chrome/browser/resources/settings/certificate_manager_page/certificate_list.js [modify] https://crrev.com/d54ece4d6be085d618f0210036a7cb373e029c49/chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.js [modify] https://crrev.com/d54ece4d6be085d618f0210036a7cb373e029c49/chrome/browser/resources/settings/certificate_manager_page/certificate_manager_types.js [modify] https://crrev.com/d54ece4d6be085d618f0210036a7cb373e029c49/chrome/browser/resources/settings/certificate_manager_page/certificate_password_decryption_dialog.js [modify] https://crrev.com/d54ece4d6be085d618f0210036a7cb373e029c49/chrome/browser/resources/settings/certificate_manager_page/certificate_password_encryption_dialog.js [modify] https://crrev.com/d54ece4d6be085d618f0210036a7cb373e029c49/chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.js
,
Apr 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d54ece4d6be085d618f0210036a7cb373e029c49 commit d54ece4d6be085d618f0210036a7cb373e029c49 Author: dpapad <dpapad@chromium.org> Date: Wed Apr 05 18:50:30 2017 MD Settings: Restore focus after closing dialogs, for certificate page. BUG= 668313 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2792663002 Cr-Commit-Position: refs/heads/master@{#462161} [modify] https://crrev.com/d54ece4d6be085d618f0210036a7cb373e029c49/chrome/browser/resources/settings/certificate_manager_page/ca_trust_edit_dialog.js [modify] https://crrev.com/d54ece4d6be085d618f0210036a7cb373e029c49/chrome/browser/resources/settings/certificate_manager_page/certificate_delete_confirmation_dialog.js [modify] https://crrev.com/d54ece4d6be085d618f0210036a7cb373e029c49/chrome/browser/resources/settings/certificate_manager_page/certificate_list.js [modify] https://crrev.com/d54ece4d6be085d618f0210036a7cb373e029c49/chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.js [modify] https://crrev.com/d54ece4d6be085d618f0210036a7cb373e029c49/chrome/browser/resources/settings/certificate_manager_page/certificate_manager_types.js [modify] https://crrev.com/d54ece4d6be085d618f0210036a7cb373e029c49/chrome/browser/resources/settings/certificate_manager_page/certificate_password_decryption_dialog.js [modify] https://crrev.com/d54ece4d6be085d618f0210036a7cb373e029c49/chrome/browser/resources/settings/certificate_manager_page/certificate_password_encryption_dialog.js [modify] https://crrev.com/d54ece4d6be085d618f0210036a7cb373e029c49/chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.js
,
Apr 6 2017
Status update. All non-ChromeOS dialogs have been updated such that focus is restored when they are closed. [DONE] passwords_and_forms_page/credit_card_edit_dialog.html passwords_and_forms_page/password_edit_dialog.html passwords_and_forms_page/address_edit_dialog.html people_page/import_data_dialog.html site_settings/add_site_dialog.html site_settings/edit_exception_dialog.html site_settings/site_data.html site_settings/site_details.html search_engines_page/search_engine_dialog.html certificate_manager_page/certificate_password_encryption_dialog.html certificate_manager_page/certificate_password_decryption_dialog.html certificate_manager_page/ca_trust_edit_dialog.html certificate_manager_page/certificate_delete_confirmation_dialog.html certificate_manager_page/certificates_error_dialog.html on_startup_page/startup_url_dialog.html reset_page/powerwash_dialog.html reset_page/reset_profile_banner.html reset_page/reset_profile_dialog.html languages_page/add_languages_dialog.html clear_browsing_data_dialog/clear_browsing_data_dialog.html people_page/people_page.html privacy_page/privacy_page.html clear_browsing_data_dialog/history_deletion_dialog.html [CL in review] about_page/channel_switcher_dialog.html https://codereview.chromium.org/2803853002 [Not done, ChromeOS] people_page/users_add_user_dialog.html people_page/easy_unlock_turn_off_dialog.html device_page/display_overscan_dialog.html device_page/drive_cache_dialog.html android_apps_page/android_apps_page.html bluetooth_page/bluetooth_device_dialog.html internet_page/network_proxy.html internet_page/network_siminfo.html internet_page/tether_connection_dialog.html people_page/password_prompt_dialog.html people_page/setup_fingerprint_dialog.html people_page/setup_pin_dialog.html printing_page/cups_add_printer_dialog_util.html I am pausing work on this to look for higher priority launch blocking non-CrOS related issues first. @stevenjb: Depending on what else is there left to do for Desktop launch, I might not get back to this anytime soon. I'll probably close this issue and file a new one to track the ChromeOS remaining tasks.
,
Apr 6 2017
Sure, that's fine. You can go ahead and assign the cros specific one to me, just please reference this issue so that I can look at some examples. I might not get to the audit / any fixes until post branch point, but I can definitely get to it before 59 Beta.
,
Apr 6 2017
|
|||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||
Comment 1 by dbeam@chromium.org
, Nov 29 2016