[MD settings] set route on advanced page toggle

[MD settings] set route on advanced page toggle

This is a fix to set the route properly
when going to basic or advanced pages
(without going to a section within
the page).

BUG=593989
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/13892b7ca9ef20f61bbd609476a72d4b6fdd3ff9
Cr-Commit-Position: refs/heads/master@{#403565}

[dschuyler, 2016-06-30 17:36:19]

Description was changed from

==========
[MD settings] set route on advanced page toggle

BUG=
==========

to

==========
[MD settings] set route on advanced page toggle

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

[dschuyler, 2016-06-30 18:26:01]

Description was changed from

==========
[MD settings] set route on advanced page toggle

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

to

==========
[MD settings] set route on advanced page toggle

This is a fix to set the route properly
when going to basic or advanced pages 
(without going to a section within
the page).

BUG=593989
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[dschuyler, 2016-06-30 18:26:01]

dschuyler@chromium.org changed reviewers:
+ michaelpg@chromium.org

[Dan Beam, 2016-06-30 23:40:26]

dbeam@chromium.org changed reviewers:
+ dbeam@chromium.org

[Dan Beam, 2016-06-30 23:40:27]

https://codereview.chromium.org/2111223002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_main/settings_main.js (right):

https://codereview.chromium.org/2111223002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_main/settings_main.js:76: url: '',
where'd we land on omitting the url key as I asked in
https://codereview.chromium.org/2090753002/diff/20001/chrome/browser/resource...

?

[dschuyler, 2016-07-01 00:59:05]

Made url optional.

https://codereview.chromium.org/2111223002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_main/settings_main.js (right):

https://codereview.chromium.org/2111223002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_main/settings_main.js:76: url: '',
On 2016/06/30 23:40:26, Dan Beam wrote:
> where'd we land on omitting the url key as I asked in
>
https://codereview.chromium.org/2090753002/diff/20001/chrome/browser/resource...
> 
> ?

Done.

[Dan Beam, 2016-07-01 01:11:51]

lgtm either way

https://codereview.chromium.org/2111223002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_main/settings_main.js (right):

https://codereview.chromium.org/2111223002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/settings_main/settings_main.js:74: section:
'',
I assume the section should be '' and not 'privacy', right?

https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/settin...
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/settin...

https://codereview.chromium.org/2111223002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_page/settings_router.js (right):

https://codereview.chromium.org/2111223002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/settings_page/settings_router.js:11: *   url:
(string|undefined),
can you just remove url from this?  and we make a type that extends this or
something?  if only the router really knows or uses URLs, we should make it
private to this class.

record types must only satisfy the fields listed in a @typedef to be considered
that type, but can have extra.

[dschuyler, 2016-07-01 22:31:22]

https://codereview.chromium.org/2111223002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_main/settings_main.js (right):

https://codereview.chromium.org/2111223002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/settings_main/settings_main.js:74: section:
'',
On 2016/07/01 01:11:51, Dan Beam wrote:
> I assume the section should be '' and not 'privacy', right?
> 
>
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/settin...
>
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/settin...

I believe so.

https://codereview.chromium.org/2111223002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_page/settings_router.js (right):

https://codereview.chromium.org/2111223002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/settings_page/settings_router.js:11: *   url:
(string|undefined),
On 2016/07/01 01:11:51, Dan Beam wrote:
> can you just remove url from this?  and we make a type that extends this or
> something?  if only the router really knows or uses URLs, we should make it
> private to this class.
> 
> record types must only satisfy the fields listed in a @typedef to be
considered
> that type, but can have extra.

Dan and I discussed this offline. Tommy is working on redoing 
the routing, so it seems best to give Tommy the room to decide 
how this should be done.

[Dan Beam, 2016-07-01 22:51:27]

fyi: I've made a CL to further clarify things regarding routing and the "url"
key

https://codereview.chromium.org/2115133002/

it's based on this one.  thanks for enabling others quickly with this CL.  also,
land this puppy :).

[dschuyler, 2016-07-01 22:53:32]

The CQ bit was checked by dschuyler@chromium.org

[commit-bot: I haz the power, 2016-07-01 22:53:42]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-01 23:51:04]

Description was changed from

==========
[MD settings] set route on advanced page toggle

This is a fix to set the route properly
when going to basic or advanced pages 
(without going to a section within
the page).

BUG=593989
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
[MD settings] set route on advanced page toggle

This is a fix to set the route properly
when going to basic or advanced pages 
(without going to a section within
the page).

BUG=593989
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-07-01 23:51:05]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-01 23:51:08]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-01 23:52:27]

Description was changed from

==========
[MD settings] set route on advanced page toggle

This is a fix to set the route properly
when going to basic or advanced pages 
(without going to a section within
the page).

BUG=593989
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
[MD settings] set route on advanced page toggle

This is a fix to set the route properly
when going to basic or advanced pages
(without going to a section within
the page).

BUG=593989
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/13892b7ca9ef20f61bbd609476a72d4b6fdd3ff9
Cr-Commit-Position: refs/heads/master@{#403565}
==========

[commit-bot: I haz the power, 2016-07-01 23:52:28]

Patchset 2 (id:??) landed as
https://crrev.com/13892b7ca9ef20f61bbd609476a72d4b6fdd3ff9
Cr-Commit-Position: refs/heads/master@{#403565}