MD Settings: OnStartup, implementing "Edit" functionality.

MD Settings: OnStartup, implementing "Edit" functionality.

 - Modifying <settings-startup-url-dialog> to act both as "Add" and "Edit".
 - Bringing up dialog when user taps the "Edit" option.
 - Adding tests.

BUG=601514
Committed: https://crrev.com/8b14059a8b62eac0b7ed74b2fc233aecb8cf8329
Cr-Commit-Position: refs/heads/master@{#387218}

[dpapad, 2016-04-12 21:43:13]

Patchset #1 (id:1) has been deleted

[dpapad, 2016-04-12 21:43:17]

Patchset #1 (id:20001) has been deleted

[dpapad, 2016-04-12 21:43:21]

Patchset #1 (id:40001) has been deleted

[dpapad, 2016-04-12 21:43:24]

Patchset #1 (id:60001) has been deleted

[dpapad, 2016-04-12 21:43:27]

Patchset #1 (id:80001) has been deleted

[dpapad, 2016-04-12 21:43:30]

Patchset #1 (id:100001) has been deleted

[dpapad, 2016-04-12 21:52:21]

Description was changed from

==========
MD Settings: OnStartup, implementing "Edit" functionality.

BUG=600887
==========

to

==========
MD Settings: OnStartup, implementing "Edit" functionality.

 - Modifying <settings-startup-url-dialog> to act both as "Add" and "Edit".
 - Bringing up dialog when user taps the "Edit" option.
 - Adding tests.

BUG=600887
==========

[dpapad, 2016-04-12 21:53:06]

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

[dpapad, 2016-04-12 21:53:07]

Screenshots at http://imgur.com/a/e5Z1A.

[Dan Beam, 2016-04-13 22:52:41]

Description was changed from

==========
MD Settings: OnStartup, implementing "Edit" functionality.

 - Modifying <settings-startup-url-dialog> to act both as "Add" and "Edit".
 - Bringing up dialog when user taps the "Edit" option.
 - Adding tests.

BUG=600887
==========

to

==========
MD Settings: OnStartup, implementing "Edit" functionality.

 - Modifying <settings-startup-url-dialog> to act both as "Add" and "Edit".
 - Bringing up dialog when user taps the "Edit" option.
 - Adding tests.

BUG=600887
==========

[Dan Beam, 2016-04-13 22:52:41]

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

[Dan Beam, 2016-04-13 22:52:52]

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

[Dan Beam, 2016-04-13 22:52:53]

-dbeam, +dschuyler

[dschuyler, 2016-04-14 00:13:49]

On 2016/04/13 22:52:53, Dan Beam wrote:
> -dbeam, +dschuyler

It looks like the bug number may be incorrect in the
CL description.

[dpapad, 2016-04-14 00:17:10]

Description was changed from

==========
MD Settings: OnStartup, implementing "Edit" functionality.

 - Modifying <settings-startup-url-dialog> to act both as "Add" and "Edit".
 - Bringing up dialog when user taps the "Edit" option.
 - Adding tests.

BUG=600887
==========

to

==========
MD Settings: OnStartup, implementing "Edit" functionality.

 - Modifying <settings-startup-url-dialog> to act both as "Add" and "Edit".
 - Bringing up dialog when user taps the "Edit" option.
 - Adding tests.

BUG=601514
==========

[dpapad, 2016-04-14 00:17:33]

On 2016/04/14 at 00:13:49, dschuyler wrote:
> On 2016/04/13 22:52:53, Dan Beam wrote:
> > -dbeam, +dschuyler
> 
> It looks like the bug number may be incorrect in the
> CL description.

Ooops. Fixed bug number.

[dschuyler, 2016-04-14 00:49:39]

https://codereview.chromium.org/1882483002/diff/130001/chrome/browser/resourc...
File chrome/browser/resources/settings/on_startup_page/startup_url_entry.js
(right):

https://codereview.chromium.org/1882483002/diff/130001/chrome/browser/resourc...
chrome/browser/resources/settings/on_startup_page/startup_url_entry.js:22: });
Can the above be simplified?

[dpapad, 2016-04-14 01:13:39]

https://codereview.chromium.org/1882483002/diff/130001/chrome/browser/resourc...
File chrome/browser/resources/settings/on_startup_page/startup_url_entry.js
(right):

https://codereview.chromium.org/1882483002/diff/130001/chrome/browser/resourc...
chrome/browser/resources/settings/on_startup_page/startup_url_entry.js:22: });
On 2016/04/14 at 00:49:38, dschuyler wrote:
> Can the above be simplified?

First, I added a compile target for this to ensure it compiles (see
compiled_resources2.gyp change). Then I tried to simplify it, by inlining the
string and removing the "var EditStartupUrlEvent", but compiler did not
understand that syntax.

[dschuyler, 2016-04-14 01:15:41]

lgtm

[dpapad, 2016-04-14 01:24:04]

The CQ bit was checked by dpapad@chromium.org

[commit-bot: I haz the power, 2016-04-14 01:24:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1882483002/150001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1882483002/150001

[Dan Beam, 2016-04-14 01:32:30]

https://codereview.chromium.org/1882483002/diff/130001/chrome/browser/resourc...
File chrome/browser/resources/settings/on_startup_page/startup_url_entry.js
(right):

https://codereview.chromium.org/1882483002/diff/130001/chrome/browser/resourc...
chrome/browser/resources/settings/on_startup_page/startup_url_entry.js:22: });
On 2016/04/14 01:13:39, dpapad wrote:
> On 2016/04/14 at 00:49:38, dschuyler wrote:
> > Can the above be simplified?
> 
> First, I added a compile target for this to ensure it compiles (see
> compiled_resources2.gyp change). Then I tried to simplify it, by inlining the
> string and removing the "var EditStartupUrlEvent", but compiler did not
> understand that syntax.

cr.exportPath('settings');

/** @type {string} */
setings.EditStartupUrlEvent = 'edit-startup-url';

but shouldn't this be in like FINAL_CASE?

[Dan Beam, 2016-04-14 01:34:25]

lgtm (doesn't need to block, fyi, just saying there probably is a simpler way)

[dpapad, 2016-04-14 01:41:56]

The CQ bit was unchecked by dpapad@chromium.org

[dpapad, 2016-04-14 01:48:27]

Patchset #4 (id:170001) has been deleted

[dpapad, 2016-04-14 01:50:53]

https://codereview.chromium.org/1882483002/diff/130001/chrome/browser/resourc...
File chrome/browser/resources/settings/on_startup_page/startup_url_entry.js
(right):

https://codereview.chromium.org/1882483002/diff/130001/chrome/browser/resourc...
chrome/browser/resources/settings/on_startup_page/startup_url_entry.js:22: });
On 2016/04/14 at 01:32:30, Dan Beam wrote:
> On 2016/04/14 01:13:39, dpapad wrote:
> > On 2016/04/14 at 00:49:38, dschuyler wrote:
> > > Can the above be simplified?
> > 
> > First, I added a compile target for this to ensure it compiles (see
> > compiled_resources2.gyp change). Then I tried to simplify it, by inlining
the
> > string and removing the "var EditStartupUrlEvent", but compiler did not
> > understand that syntax.
> 
> cr.exportPath('settings');
> 
> /** @type {string} */
> setings.EditStartupUrlEvent = 'edit-startup-url';
> 
> but shouldn't this be in like FINAL_CASE?

Done the simplification. Also changed to @const and FINAL_CASE

[dpapad, 2016-04-14 01:52:04]

The CQ bit was checked by dpapad@chromium.org

[dpapad, 2016-04-14 01:52:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
dschuyler@chromium.org, dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/1882483002/#ps190001
(title: "Nit.")

[commit-bot: I haz the power, 2016-04-14 01:52:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1882483002/190001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1882483002/190001

[commit-bot: I haz the power, 2016-04-14 02:55:37]

Committed patchset #4 (id:190001)

[commit-bot: I haz the power, 2016-04-14 02:56:36]

Description was changed from

==========
MD Settings: OnStartup, implementing "Edit" functionality.

 - Modifying <settings-startup-url-dialog> to act both as "Add" and "Edit".
 - Bringing up dialog when user taps the "Edit" option.
 - Adding tests.

BUG=601514
==========

to

==========
MD Settings: OnStartup, implementing "Edit" functionality.

 - Modifying <settings-startup-url-dialog> to act both as "Add" and "Edit".
 - Bringing up dialog when user taps the "Edit" option.
 - Adding tests.

BUG=601514

Committed: https://crrev.com/8b14059a8b62eac0b7ed74b2fc233aecb8cf8329
Cr-Commit-Position: refs/heads/master@{#387218}
==========

[commit-bot: I haz the power, 2016-04-14 02:56:37]

Patchset 4 (id:??) landed as
https://crrev.com/8b14059a8b62eac0b7ed74b2fc233aecb8cf8329
Cr-Commit-Position: refs/heads/master@{#387218}