arc: Add Goolge Privacy Policy link to OptIn UI.

arc: Add Goolge Privacy Policy link to OptIn UI.

This adds a link to GetStarted page that opens Privacy Policy for
the language currently selected in ToS

BUG=b/30103177
TEST=Locally on device.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/06650954785fb597dba01d710db0a8665206e647
Cr-Commit-Position: refs/heads/master@{#426288}

[khmel, 2016-10-19 15:45:04]

Description was changed from

==========
arc: Add Goolge Privacy Policy link to OptIn UI.

This adds a link to GetStarted page that opens Privacy Policy for
the language currently selected in ToS

BUG=b/30103177
TEST=Locally on device.
==========

to

==========
arc: Add Goolge Privacy Policy link to OptIn UI.

This adds a link to GetStarted page that opens Privacy Policy for
the language currently selected in ToS

BUG=b/30103177
TEST=Locally on device.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[khmel, 2016-10-19 15:47:32]

khmel@chromium.org changed reviewers:
+ xiyuan@chromium.org

[khmel, 2016-10-19 15:47:32]

Hi Xiyuan,

PTAL

[khmel, 2016-10-19 16:14:41]

Sorry, forgot to describe things that can be not obvious.

PTAL

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

https://codereview.chromium.org/2434823002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/main.html:98: <a
class="checkbox-text" id="privacy-policy-link"
We don't have enough space on GetStarted page in the area of check boxes where
we were planning to put this text originally (especially for non-US lang). So
solution is to put it to the left from the button area and use short form only
'Google Privacy Policy'

You may find how it looks now here:
https://drive.google.com/a/google.com/file/d/0B63tZNwdjs-qUmtIdzZiNmdzX28/vie...

https://codereview.chromium.org/2434823002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/arc_support/playstore.js (right):

https://codereview.chromium.org/2434823002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/playstore.js:119: var links =
document.getElementById('play-footer').getElementsByTagName('a');
Privacy Policy also has similar structure for zone/lang as PlayStore TOS. To be
consistent it seems the easiest way is to extract link from ToS

[xiyuan, 2016-10-19 16:17:51]

https://codereview.chromium.org/2434823002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/arc_support/main.css (right):

https://codereview.chromium.org/2434823002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/main.css:40: text-align: left;
Consider removing this and putting #privacy-policy-link in a block element (e.g.
<div>) with "flex" class.

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

https://codereview.chromium.org/2434823002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/main.html:100: <div class="layout
flex auto"></div>
What is this <div> for? To make the privacy link show on left? I would prefer to
get this work than the text-align css rule.

Would this

<div class="flex"><a id="privacy-policy-link"...></a></div>

work?

[xiyuan, 2016-10-19 16:21:31]

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

https://codereview.chromium.org/2434823002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/main.html:98: <a
class="checkbox-text" id="privacy-policy-link"
On 2016/10/19 16:14:41, khmel wrote:
> We don't have enough space on GetStarted page in the area of check boxes where
> we were planning to put this text originally (especially for non-US lang). So
> solution is to put it to the left from the button area and use short form only
> 'Google Privacy Policy'
> 
> You may find how it looks now here:
>
https://drive.google.com/a/google.com/file/d/0B63tZNwdjs-qUmtIdzZiNmdzX28/vie...

The privacy link looks vertical top aligned. Maybe add a "center" class to the
button strip div?

[khmel, 2016-10-19 16:33:30]

Thank you for review.

Please see my answers.

https://codereview.chromium.org/2434823002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/arc_support/main.css (right):

https://codereview.chromium.org/2434823002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/main.css:40: text-align: left;
On 2016/10/19 16:17:51, xiyuan wrote:
> Consider removing this and putting #privacy-policy-link in a block element
(e.g.
> <div>) with "flex" class.

I removed it. It is not required in either case. Also commented about div in
html file.

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

https://codereview.chromium.org/2434823002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/main.html:98: <a
class="checkbox-text" id="privacy-policy-link"
On 2016/10/19 16:21:30, xiyuan wrote:
> On 2016/10/19 16:14:41, khmel wrote:
> > We don't have enough space on GetStarted page in the area of check boxes
where
> > we were planning to put this text originally (especially for non-US lang).
So
> > solution is to put it to the left from the button area and use short form
only
> > 'Google Privacy Policy'
> > 
> > You may find how it looks now here:
> >
>
https://drive.google.com/a/google.com/file/d/0B63tZNwdjs-qUmtIdzZiNmdzX28/vie...
> 
> The privacy link looks vertical top aligned. Maybe add a "center" class to the
> button strip div?

I don't have exact specs how to place it (just 'place it here'). I did it
intentionally but agree that with center layout it looks better.

https://codereview.chromium.org/2434823002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/main.html:100: <div class="layout
flex auto"></div>
On 2016/10/19 16:17:51, xiyuan wrote:
> What is this <div> for? To make the privacy link show on left? I would prefer
to
> get this work than the text-align css rule.
> 
> Would this
> 
> <div class="flex"><a id="privacy-policy-link"...></a></div>
> 
> work?

It works not as expected, please see :
https://drive.google.com/a/google.com/file/d/0B63tZNwdjs-qQk9ReFdzdXhjYms/vie...

My attempt was to do following layout:

link grab_free_space_by_empty_div button separator button

[xiyuan, 2016-10-19 16:55:29]

lgtm

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

https://codereview.chromium.org/2434823002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/main.html:100: <div class="layout
flex auto"></div>
On 2016/10/19 16:33:30, khmel wrote:
> On 2016/10/19 16:17:51, xiyuan wrote:
> > What is this <div> for? To make the privacy link show on left? I would
prefer
> to
> > get this work than the text-align css rule.
> > 
> > Would this
> > 
> > <div class="flex"><a id="privacy-policy-link"...></a></div>
> > 
> > work?
> 
> It works not as expected, please see :
>
https://drive.google.com/a/google.com/file/d/0B63tZNwdjs-qQk9ReFdzdXhjYms/vie...
> 
> My attempt was to do following layout:
> 
> link grab_free_space_by_empty_div button separator button

I see. So you are using it as a padding div.

https://codereview.chromium.org/2434823002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/chromeos/arc_support/main.html (right):

https://codereview.chromium.org/2434823002/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/arc_support/main.html:100: <div class="layout
flex auto"></div>
nit: class "layout" and "out" are probably not needed.

[khmel, 2016-10-19 18:19:28]

Thank you!

https://codereview.chromium.org/2434823002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/chromeos/arc_support/main.html (right):

https://codereview.chromium.org/2434823002/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/arc_support/main.html:100: <div class="layout
flex auto"></div>
On 2016/10/19 16:55:29, xiyuan wrote:
> nit: class "layout" and "out" are probably not needed.

Right

[khmel, 2016-10-19 18:19:32]

The CQ bit was checked by khmel@chromium.org

[khmel, 2016-10-19 18:19:32]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org
Link to the patchset: https://codereview.chromium.org/2434823002/#ps40001
(title: "remove not-needed styles")

[commit-bot: I haz the power, 2016-10-19 18:20:07]

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-10-19 19:10:25]

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

[commit-bot: I haz the power, 2016-10-19 19:10:26]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[khmel, 2016-10-19 19:45:47]

The CQ bit was checked by khmel@chromium.org

[khmel, 2016-10-19 19:45:47]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org
Link to the patchset: https://codereview.chromium.org/2434823002/#ps60001
(title: "comments updated, make presubmit passed")

[commit-bot: I haz the power, 2016-10-19 19:46:55]

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-10-19 21:26:35]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-21 13:11:34]

Description was changed from

==========
arc: Add Goolge Privacy Policy link to OptIn UI.

This adds a link to GetStarted page that opens Privacy Policy for
the language currently selected in ToS

BUG=b/30103177
TEST=Locally on device.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
arc: Add Goolge Privacy Policy link to OptIn UI.

This adds a link to GetStarted page that opens Privacy Policy for
the language currently selected in ToS

BUG=b/30103177
TEST=Locally on device.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/06650954785fb597dba01d710db0a8665206e647
Cr-Commit-Position: refs/heads/master@{#426288}
==========

[commit-bot: I haz the power, 2016-10-21 13:11:35]

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