Settings People Sync: Add 'Reset Sync' message for forgotten passphrase.

Settings People Sync: Add 'Reset Sync' message for forgotten passphrase.

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

Committed: https://crrev.com/670bf00141a20f37d7208fc54a6429004d73b360
Cr-Commit-Position: refs/heads/master@{#412908}

[tommycli, 2016-08-16 23:36:10]

Description was changed from

==========
Settings People Sync: Add 'Reset Sync' message for forgotten passphrase.

BUG=627595
==========

to

==========
Settings People Sync: Add 'Reset Sync' message for forgotten passphrase.

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

[tommycli, 2016-08-16 23:36:46]

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

[tommycli, 2016-08-16 23:36:47]

dschuyler: PTAL

Adds the message at the bottom of this screenie:
http://imgur.com/a/m3fVW

[tommycli, 2016-08-17 21:58:58]

On 2016/08/16 23:36:47, tommycli wrote:
> dschuyler: PTAL
> 
> Adds the message at the bottom of this screenie:
> http://imgur.com/a/m3fVW

dschuyler: ping

[dschuyler, 2016-08-18 01:09:29]

https://codereview.chromium.org/2250733003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/people_page/sync_page.html (right):

https://codereview.chromium.org/2250733003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/people_page/sync_page.html:205:
<span>$i18nRaw{passphraseRecover}</span>
nit: is the span needed?

https://codereview.chromium.org/2250733003/diff/1/chrome/browser/ui/webui/set...
File chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
(right):

https://codereview.chromium.org/2250733003/diff/1/chrome/browser/ui/webui/set...
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:968:
html_source->AddString("syncDashboardUrl", sync_dashboard_url);
Sadly, sync_dashboard_url will be turned back
into a std::string in AddString().

https://cs.chromium.org/chromium/src/content/browser/webui/web_ui_data_source...

Dunno if this really matters, since it will need 
to be a utf16 for the GetStringFUTF16(). There may
be nothing to change here, but I thought I'd mention
it in case you had an idea how to avoid the conversions
back and forth.

https://codereview.chromium.org/2250733003/diff/1/chrome/browser/ui/webui/set...
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:971:
l10n_util::GetStringFUTF16(IDS_SETTINGS_PASSPHRASE_RECOVER,
Not a big deal, but this can be
l10n_util::GetStringFUTF8, which
will simply do the base::UTF16ToUTF8
here rather than in AddString(), but
imo it helps others see that the 
html is really utf8 (and not utf16).

[dschuyler, 2016-08-18 01:28:29]

My prior comments are all optional or FYI.
LGTM

[tommycli, 2016-08-18 16:23:31]

thanks!

https://codereview.chromium.org/2250733003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/people_page/sync_page.html (right):

https://codereview.chromium.org/2250733003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/people_page/sync_page.html:205:
<span>$i18nRaw{passphraseRecover}</span>
On 2016/08/18 01:09:29, dschuyler wrote:
> nit: is the span needed?

Yes, because list-item is display:flex, which messes up the text. There's no
.list-item.block rule like with settings-box.

https://codereview.chromium.org/2250733003/diff/1/chrome/browser/ui/webui/set...
File chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
(right):

https://codereview.chromium.org/2250733003/diff/1/chrome/browser/ui/webui/set...
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:968:
html_source->AddString("syncDashboardUrl", sync_dashboard_url);
On 2016/08/18 01:09:29, dschuyler wrote:
> Sadly, sync_dashboard_url will be turned back
> into a std::string in AddString().
> 
>
https://cs.chromium.org/chromium/src/content/browser/webui/web_ui_data_source...
> 
> Dunno if this really matters, since it will need 
> to be a utf16 for the GetStringFUTF16(). There may
> be nothing to change here, but I thought I'd mention
> it in case you had an idea how to avoid the conversions
> back and forth.

Done.

https://codereview.chromium.org/2250733003/diff/1/chrome/browser/ui/webui/set...
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:971:
l10n_util::GetStringFUTF16(IDS_SETTINGS_PASSPHRASE_RECOVER,
On 2016/08/18 01:09:29, dschuyler wrote:
> Not a big deal, but this can be
> l10n_util::GetStringFUTF8, which
> will simply do the base::UTF16ToUTF8
> here rather than in AddString(), but
> imo it helps others see that the 
> html is really utf8 (and not utf16).

Done.

[tommycli, 2016-08-18 16:23:35]

The CQ bit was checked by tommycli@chromium.org

[tommycli, 2016-08-18 16:23:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
dschuyler@chromium.org
Link to the patchset: https://codereview.chromium.org/2250733003/#ps20001
(title: "address comments")

[commit-bot: I haz the power, 2016-08-18 16:23:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-18 16:23:55]

This CL has an open dependency (Issue 2248973004 Patch 1). Please resolve the
dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[tommycli, 2016-08-18 16:59:57]

The CQ bit was checked by tommycli@chromium.org

[commit-bot: I haz the power, 2016-08-18 17:00:35]

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-08-18 19:16:11]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-18 19:18:27]

Description was changed from

==========
Settings People Sync: Add 'Reset Sync' message for forgotten passphrase.

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

to

==========
Settings People Sync: Add 'Reset Sync' message for forgotten passphrase.

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

Committed: https://crrev.com/670bf00141a20f37d7208fc54a6429004d73b360
Cr-Commit-Position: refs/heads/master@{#412908}
==========

[commit-bot: I haz the power, 2016-08-18 19:18:28]

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