[i18n] chromeos first run, i18n-content to $i18n{}

[i18n] chromeos first run, i18n-content to $i18n{}

This is a mechanical change to convert i18n-content to $i18n{} tags.

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

Review-Url: https://codereview.chromium.org/2625873006
Cr-Commit-Position: refs/heads/master@{#443108}
Committed: https://chromium.googlesource.com/chromium/src/+/2b7b8dafe5412ef81b6c4755bdbf700bd55d138a

[dschuyler, 2017-01-11 23:03:36]

Description was changed from

==========
[i18n] chromeos first run, i18n-content to $i18n{}

This is a mechanical change to convert i18n-content to $i18n{} tags.

BUG=677338
==========

to

==========
[i18n] chromeos first run, i18n-content to $i18n{}

This is a mechanical change to convert i18n-content to $i18n{} tags.

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

[dschuyler, 2017-01-11 23:03:38]

The CQ bit was checked by dschuyler@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 23:04:35]

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

[dschuyler, 2017-01-11 23:06:12]

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

[dschuyler, 2017-01-11 23:06:13]

https://codereview.chromium.org/2625873006/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/first_run/app/main.html (right):

https://codereview.chromium.org/2625873006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/first_run/app/main.html:33: </p>
I'm intentionally resisting the urge to change this <p>, <span>, and <br> so
that this stays as a mechanical change.

[commit-bot: I haz the power, 2017-01-12 00:36:38]

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

[commit-bot: I haz the power, 2017-01-12 00:36:39]

Dry run: This issue passed the CQ dry run.

[tommycli, 2017-01-12 00:53:14]

https://codereview.chromium.org/2625873006/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/first_run/app/main.html (right):

https://codereview.chromium.org/2625873006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/first_run/app/main.html:33: </p>
On 2017/01/11 23:06:13, dschuyler wrote:
> I'm intentionally resisting the urge to change this <p>, <span>, and <br> so
> that this stays as a mechanical change.

Well it looks like the only purpose of those spans before was to hold the
i18n-content. I worry future devs are going to look at this and wonder why these
spans exist.

[dschuyler, 2017-01-12 01:06:54]

https://codereview.chromium.org/2625873006/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/first_run/app/main.html (right):

https://codereview.chromium.org/2625873006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/first_run/app/main.html:33: </p>
On 2017/01/12 00:53:14, tommycli wrote:
> On 2017/01/11 23:06:13, dschuyler wrote:
> > I'm intentionally resisting the urge to change this <p>, <span>, and <br> so
> > that this stays as a mechanical change.
> 
> Well it looks like the only purpose of those spans before was to hold the
> i18n-content. I worry future devs are going to look at this and wonder why
these
> spans exist.

It's a good worry. My worry is that if the changes happen together, the i18n
conversion will be reverted to fix a potential layout issue.

It's not trivial (doable, just not trivial) to prove that none of those elements
have unexpected styling by one of the many includes (ya can't just glance at
them and know that it's ok to pull them out). It's also hard to know whether a
UI designer was particular about the exact layout of these lines (the layout may
shift very slightly if these are changed).

My preference would be to do the changes in separate CLs so that if someone does
a git bisect to find out where layout changed, the i18n replacement won't be
compounded in the change. i.e. may I do it in a follow-up CL?

[tommycli, 2017-01-12 01:13:13]

On 2017/01/12 01:06:54, dschuyler wrote:
>
https://codereview.chromium.org/2625873006/diff/1/chrome/browser/resources/ch...
> File chrome/browser/resources/chromeos/first_run/app/main.html (right):
> 
>
https://codereview.chromium.org/2625873006/diff/1/chrome/browser/resources/ch...
> chrome/browser/resources/chromeos/first_run/app/main.html:33: </p>
> On 2017/01/12 00:53:14, tommycli wrote:
> > On 2017/01/11 23:06:13, dschuyler wrote:
> > > I'm intentionally resisting the urge to change this <p>, <span>, and <br>
so
> > > that this stays as a mechanical change.
> > 
> > Well it looks like the only purpose of those spans before was to hold the
> > i18n-content. I worry future devs are going to look at this and wonder why
> these
> > spans exist.
> 
> It's a good worry. My worry is that if the changes happen together, the i18n
> conversion will be reverted to fix a potential layout issue.
> 
> It's not trivial (doable, just not trivial) to prove that none of those
elements
> have unexpected styling by one of the many includes (ya can't just glance at
> them and know that it's ok to pull them out). It's also hard to know whether a
> UI designer was particular about the exact layout of these lines (the layout
may
> shift very slightly if these are changed).
> 
> My preference would be to do the changes in separate CLs so that if someone
does
> a git bisect to find out where layout changed, the i18n replacement won't be
> compounded in the change. i.e. may I do it in a follow-up CL?

I see what you're saying about the layout issues. lgtm to doing it in a followup
CL

[dschuyler, 2017-01-12 01:18:26]

The CQ bit was checked by dschuyler@chromium.org

[commit-bot: I haz the power, 2017-01-12 01:19:20]

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, 2017-01-12 01:27:35]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1484183906358440, "parent_rev":
"8e036fa110c1aa1f40aa536f3a2d14f1d78b6c30", "commit_rev":
"2b7b8dafe5412ef81b6c4755bdbf700bd55d138a"}

[commit-bot: I haz the power, 2017-01-12 01:28:05]

Description was changed from

==========
[i18n] chromeos first run, i18n-content to $i18n{}

This is a mechanical change to convert i18n-content to $i18n{} tags.

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

to

==========
[i18n] chromeos first run, i18n-content to $i18n{}

This is a mechanical change to convert i18n-content to $i18n{} tags.

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

Review-Url: https://codereview.chromium.org/2625873006
Cr-Commit-Position: refs/heads/master@{#443108}
Committed:
https://chromium.googlesource.com/chromium/src/+/2b7b8dafe5412ef81b6c4755bdbf...
==========

[commit-bot: I haz the power, 2017-01-12 01:28:06]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/2b7b8dafe5412ef81b6c4755bdbf...

[dschuyler, 2017-01-12 21:51:54]

On 2017/01/12 01:13:13, tommycli wrote:
> On 2017/01/12 01:06:54, dschuyler wrote:
> >
>
https://codereview.chromium.org/2625873006/diff/1/chrome/browser/resources/ch...
> > File chrome/browser/resources/chromeos/first_run/app/main.html (right):
> > 
> >
>
https://codereview.chromium.org/2625873006/diff/1/chrome/browser/resources/ch...
> > chrome/browser/resources/chromeos/first_run/app/main.html:33: </p>
> > On 2017/01/12 00:53:14, tommycli wrote:
> > > On 2017/01/11 23:06:13, dschuyler wrote:
> > > > I'm intentionally resisting the urge to change this <p>, <span>, and
<br>
> so
> > > > that this stays as a mechanical change.
> > > 
> > > Well it looks like the only purpose of those spans before was to hold the
> > > i18n-content. I worry future devs are going to look at this and wonder why
> > these
> > > spans exist.
> > 
> > It's a good worry. My worry is that if the changes happen together, the i18n
> > conversion will be reverted to fix a potential layout issue.
> > 
> > It's not trivial (doable, just not trivial) to prove that none of those
> elements
> > have unexpected styling by one of the many includes (ya can't just glance at
> > them and know that it's ok to pull them out). It's also hard to know whether
a
> > UI designer was particular about the exact layout of these lines (the layout
> may
> > shift very slightly if these are changed).
> > 
> > My preference would be to do the changes in separate CLs so that if someone
> does
> > a git bisect to find out where layout changed, the i18n replacement won't be
> > compounded in the change. i.e. may I do it in a follow-up CL?
> 
> I see what you're saying about the layout issues. lgtm to doing it in a
followup
> CL

See CL https://codereview.chromium.org/2625053007/

[dschuyler, 2017-01-25 01:31:15]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2653123003/ by dschuyler@chromium.org.

The reason for reverting is: This CL appears to have brought about this issue:
https://bugs.chromium.org/p/chromium/issues/detail?id=684654&q=owner%3Ame&col...

I'd like to revert this as a quick fix and then look into the issue further at a
later time..