New issue
Advanced search Search tips

Issue 709586 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 671375



Sign in to add a comment

MD Settings; Exiting a subpage should restore focus to the subpage trigger (arrow icon).

Project Member Reported by dpa...@chromium.org, Apr 7 2017

Issue description

Currently the focus is is transferred to an unexpected location (depends on DOM structure). Instead the focus should match the user's expectations.

Candidate approach is at https://codereview.chromium.org/2804543005. Need to figure out how to make this functionality more scale-able (instead of copy pasting it in several places).
 

Comment 1 by dbeam@chromium.org, Apr 7 2017

Blocking: 671375
Labels: -Pri-2 Pri-1

Comment 2 by dbeam@chromium.org, Apr 7 2017

Cc: lpalmaro@chromium.org
Labels: Hotlist-MD-Settings-PageA11y
Owner: dpa...@chromium.org
Status: Started (was: Untriaged)
I updated my approach based on my conversation with @tommycli, see https://codereview.chromium.org/2805363002. In short, the logic about focusing when a subpage exits resides in settings_animated_pages.js. Users of settings-animated-pages simply need to pass a config object that associates route paths with elements to be focused.

@tommycli: I ended up not adding a getPreviousRoute() method in route.js, because settings_animated_pages.js is already implements RouteObserverBehavior.

Let me know about WDYT.

Comment 4 by dbeam@chromium.org, Apr 8 2017

i like the focusConfig approach a bunch.  let the rest of the team know how/if they can help.
Thanks. Will do as soon as the CL that adds the focusConfig mechanism lands.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 11 2017

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

commit 5c96df5fc329e084e0d7752bf23f8a12e6acd344
Author: dpapad <dpapad@chromium.org>
Date: Tue Apr 11 02:42:19 2017

MD Settings: Add mechanism to restore focus after subpage exiting.

BUG= 709586 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2805363002
Cr-Commit-Position: refs/heads/master@{#463502}

[modify] https://crrev.com/5c96df5fc329e084e0d7752bf23f8a12e6acd344/chrome/browser/resources/settings/passwords_and_forms_page/passwords_and_forms_page.html
[modify] https://crrev.com/5c96df5fc329e084e0d7752bf23f8a12e6acd344/chrome/browser/resources/settings/passwords_and_forms_page/passwords_and_forms_page.js
[modify] https://crrev.com/5c96df5fc329e084e0d7752bf23f8a12e6acd344/chrome/browser/resources/settings/search_page/search_page.html
[modify] https://crrev.com/5c96df5fc329e084e0d7752bf23f8a12e6acd344/chrome/browser/resources/settings/search_page/search_page.js
[modify] https://crrev.com/5c96df5fc329e084e0d7752bf23f8a12e6acd344/chrome/browser/resources/settings/settings_page/settings_animated_pages.html
[modify] https://crrev.com/5c96df5fc329e084e0d7752bf23f8a12e6acd344/chrome/browser/resources/settings/settings_page/settings_animated_pages.js
[modify] https://crrev.com/5c96df5fc329e084e0d7752bf23f8a12e6acd344/chrome/test/data/webui/settings/cr_settings_interactive_ui_tests.js
[add] https://crrev.com/5c96df5fc329e084e0d7752bf23f8a12e6acd344/chrome/test/data/webui/settings/settings_animated_pages_test.js

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 11 2017

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

commit a4375e001388a81979b761bfb4fa60b572d0ecc8
Author: dpapad <dpapad@chromium.org>
Date: Tue Apr 11 04:07:49 2017

MD Settings: Restore focus after exiting various subpages.

Addressing subpages residing in
 - a11y_page
 - appearance_page
 - languages_page
 - people_page
 - printing_page (except CUPS printer sub-sub page).

BUG= 709586 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2811693004
Cr-Commit-Position: refs/heads/master@{#463527}

[modify] https://crrev.com/a4375e001388a81979b761bfb4fa60b572d0ecc8/chrome/browser/resources/settings/a11y_page/a11y_page.html
[modify] https://crrev.com/a4375e001388a81979b761bfb4fa60b572d0ecc8/chrome/browser/resources/settings/a11y_page/a11y_page.js
[modify] https://crrev.com/a4375e001388a81979b761bfb4fa60b572d0ecc8/chrome/browser/resources/settings/appearance_page/appearance_page.html
[modify] https://crrev.com/a4375e001388a81979b761bfb4fa60b572d0ecc8/chrome/browser/resources/settings/appearance_page/appearance_page.js
[modify] https://crrev.com/a4375e001388a81979b761bfb4fa60b572d0ecc8/chrome/browser/resources/settings/languages_page/languages_page.html
[modify] https://crrev.com/a4375e001388a81979b761bfb4fa60b572d0ecc8/chrome/browser/resources/settings/languages_page/languages_page.js
[modify] https://crrev.com/a4375e001388a81979b761bfb4fa60b572d0ecc8/chrome/browser/resources/settings/people_page/people_page.html
[modify] https://crrev.com/a4375e001388a81979b761bfb4fa60b572d0ecc8/chrome/browser/resources/settings/people_page/people_page.js
[modify] https://crrev.com/a4375e001388a81979b761bfb4fa60b572d0ecc8/chrome/browser/resources/settings/printing_page/printing_page.html
[modify] https://crrev.com/a4375e001388a81979b761bfb4fa60b572d0ecc8/chrome/browser/resources/settings/printing_page/printing_page.js
[modify] https://crrev.com/a4375e001388a81979b761bfb4fa60b572d0ecc8/chrome/test/data/webui/settings/cr_settings_browsertest.js

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 11 2017

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

commit 81f6e86468ad7d5b28e2ab17fff1c2a85422fc0a
Author: dpapad <dpapad@chromium.org>
Date: Tue Apr 11 22:18:56 2017

MD Settings: Restore focus when exiting various subpages, part 2.

Addressing subpages residing in
 - about_page
 - android_apps_page
 - privacy_page (except site settings sub-subpages).
 - device_page

BUG= 709586 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2811573004
Cr-Commit-Position: refs/heads/master@{#463800}

[modify] https://crrev.com/81f6e86468ad7d5b28e2ab17fff1c2a85422fc0a/chrome/browser/resources/settings/about_page/about_page.html
[modify] https://crrev.com/81f6e86468ad7d5b28e2ab17fff1c2a85422fc0a/chrome/browser/resources/settings/about_page/about_page.js
[modify] https://crrev.com/81f6e86468ad7d5b28e2ab17fff1c2a85422fc0a/chrome/browser/resources/settings/android_apps_page/android_apps_page.html
[modify] https://crrev.com/81f6e86468ad7d5b28e2ab17fff1c2a85422fc0a/chrome/browser/resources/settings/android_apps_page/android_apps_page.js
[modify] https://crrev.com/81f6e86468ad7d5b28e2ab17fff1c2a85422fc0a/chrome/browser/resources/settings/device_page/device_page.html
[modify] https://crrev.com/81f6e86468ad7d5b28e2ab17fff1c2a85422fc0a/chrome/browser/resources/settings/device_page/device_page.js
[modify] https://crrev.com/81f6e86468ad7d5b28e2ab17fff1c2a85422fc0a/chrome/browser/resources/settings/privacy_page/privacy_page.html
[modify] https://crrev.com/81f6e86468ad7d5b28e2ab17fff1c2a85422fc0a/chrome/browser/resources/settings/privacy_page/privacy_page.js

Status update: I have addressed sub-pages residing in the following files:

a11y_page/a11y_page.html                                                                                                                                                                                            
about_page/about_page.html
android_apps_page/android_apps_page.html
appearance_page/appearance_page.html
device_page/device_page.html
languages_page/languages_page.html
passwords_and_forms_page/passwords_and_forms_page.html
people_page/people_page.html
printing_page/printing_page.html
privacy_page/privacy_page.html
privacy_page/privacy_page.html
search_page/search_page.html
site_settings_page/site_settings_page.html

Remaining cases for Desktop
site_settings_page cookie details sub-subpage (see screencast).

Remaining cases for CrOS
printing_page/printing_page.html (cups printer details sub-subpage)
bluetooth_page/bluetooth_page.html
internet_page/internet_page.html

cookie_details_supbage_focus.mp4
517 KB View Download
Cc: steve...@chromium.org

Comment 13 by dbeam@chromium.org, Apr 20 2017

hey dpapad@ and stevenjb@: what's left to do here?
The only remaining cases I am aware of are for CrOS, specifically:

printing_page/printing_page.html (cups printer details sub-subpage)
bluetooth_page/bluetooth_page.html
internet_page/internet_page.html
Status: Fixed (was: Started)
Marking this as fixed. Filed  issue 714350  for the remaining CrOS cases.

Sign in to add a comment