New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 763186 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug
M62



Sign in to add a comment

[MD settings] move OnStartup UI from sub-page to basic page

Project Member Reported by dschuyler@chromium.org, Sep 8 2017

Issue description

in chrome://settings/onStartup
we'd previously moved to the OnStartup UI from the main (basic) page to a sub-page to avoid rendering/animation issues with UI changing on the main page.

bettes@ requested that we revert back to the prior UI where the OnStartup UI is on the main page.
 
Owner: tbuck...@chromium.org
Tom, please make me owner if you'd like this to happen.
It will require a merge into m62 as we are past the m62 branch.
Tom, also see email thread "Moving part of OnStartup back to basic page".
Labels: Proj-MaterialDesign-WebUI
Owner: dschuyler@chromium.org
Yes, let's merge this into M62.

Note that the change was made in M61, so we are acknowledging that users will see:
M60: top-level
M61: subpage
M62: top-level (after this merge)
That doesn't sound like a great user experience since we are planning to change the UI again* (likely in m63).

*because having this UI at the top-level recreates issues that were being addressed by moving it to a sub-page, so we'd still need to address dynamic content changing the page while the page is opening.
*because having this UI at the top-level recreates issues that were being addressed by moving it to a sub-page, so we'd still need to address dynamic content changing the page while the page is opening.

Understood, but from a user's perspective, I think the benefits of having their basic controls accessible on the top-level is more important than the dynamic content issue. 
Status: Started (was: Assigned)
CL at https://chromium-review.googlesource.com/c/chromium/src/+/656258
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 9 2017

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

commit c9a3169d206e066bf33f3b4e399cce5ac0782c61
Author: Dave Schuyler <dschuyler@chromium.org>
Date: Sat Sep 09 01:32:11 2017

[MD settings] move on startup UI from sub-page to basic page

This CL moves the OnStartup UI from a sub-page back to the main (basic)
page. This essentially undoes (revert) CLs 557971 and 582467 while keeping the interim changes/fixes.

Bug:  763186 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Idabfd1f2766c6bebaa1bc0ac72e3e7412af1ea8f
Reviewed-on: https://chromium-review.googlesource.com/656258
Reviewed-by: Scott Chen <scottchen@chromium.org>
Commit-Queue: Dave Schuyler <dschuyler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500772}
[modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/app/settings_strings.grdp
[modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/browser/resources/settings/on_startup_page/compiled_resources2.gyp
[modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/browser/resources/settings/on_startup_page/on_startup_page.html
[modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/browser/resources/settings/on_startup_page/on_startup_page.js
[modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/browser/resources/settings/on_startup_page/startup_urls_page.html
[modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/browser/resources/settings/on_startup_page/startup_urls_page.js
[modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
[modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/test/data/webui/settings/on_startup_browsertest.js
[modify] https://crrev.com/c9a3169d206e066bf33f3b4e399cce5ac0782c61/chrome/test/data/webui/settings/startup_urls_page_test.js

Labels: Merge-Request-62
Project Member

Comment 10 by sheriffbot@chromium.org, Sep 11 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: There is .grd file changes and we are only 35 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Please add appropriate OSs.
Labels: OS-Chrome OS-Linux OS-Mac OS-Windows
candrada@, to be helpful: here are some pro / con topics for merging this CL:

pro:
- UI change desired by UX (bettes@) and PM (tbuckley@)
- scottchen@ reviewed this CL with the knowledge that this was intended for merge (and was asked to be extra attentive to it for that reason)
- This is a self contained CL and the prior code and this code are both reasonable to ship (so there's a solid fallback/rollback option here).
- No features change (only moving existing features around in the UI)

con:
- No features change (the last to 'pro' is a 'con' depending on one's point of view).


Thanks for confirming and providing more context. Can you confirm if this is already tested and verified in Canary?
I can say that I see the change in Mac OSX chrome 63.0.3212.0 canary. (though it would be great to get it independently verified).

Can someone from QA weigh in on "tested and verified in Canary?"
Labels: -Merge-Review-62 Merge-Approved-62
Confirmed over chat that this is tested in Canary and there is a quick revert option (if needed). Approving merge to M62 (branch: 3202). 
Project Member

Comment 17 by bugdroid1@chromium.org, Sep 11 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880

commit f7fe4ab71a678e23bdbe18ef9fa01459d9efe880
Author: Dave Schuyler <dschuyler@chromium.org>
Date: Mon Sep 11 23:25:38 2017

[MD settings] move on startup UI from sub-page to basic page

This CL moves the OnStartup UI from a sub-page back to the main (basic)
page. This essentially undoes (revert) CLs 557971 and 582467 while keeping the interim changes/fixes.

Bug:  763186 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Idabfd1f2766c6bebaa1bc0ac72e3e7412af1ea8f
Reviewed-on: https://chromium-review.googlesource.com/656258
Reviewed-by: Scott Chen <scottchen@chromium.org>
Commit-Queue: Dave Schuyler <dschuyler@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#500772}(cherry picked from commit c9a3169d206e066bf33f3b4e399cce5ac0782c61)
Reviewed-on: https://chromium-review.googlesource.com/661743
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#151}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/app/settings_strings.grdp
[modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/browser/resources/settings/on_startup_page/compiled_resources2.gyp
[modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/browser/resources/settings/on_startup_page/on_startup_page.html
[modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/browser/resources/settings/on_startup_page/on_startup_page.js
[modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/browser/resources/settings/on_startup_page/startup_urls_page.html
[modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/browser/resources/settings/on_startup_page/startup_urls_page.js
[modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
[modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/test/data/webui/settings/on_startup_browsertest.js
[modify] https://crrev.com/f7fe4ab71a678e23bdbe18ef9fa01459d9efe880/chrome/test/data/webui/settings/startup_urls_page_test.js

Status: Fixed (was: Started)

Sign in to add a comment