MD Settings: About page, show regulatory info, if available.

MD Settings: About page, show regulatory info, if available.

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

Committed: https://crrev.com/b8c65611ac43f8d95f2f46f91d80c0284589b64f
Cr-Commit-Position: refs/heads/master@{#395726}

[dpapad, 2016-05-23 22:49:00]

Description was changed from

==========
MD Settings: About page, show regulatory info, if available.

BUG=546841
==========

to

==========
MD Settings: About page, show regulatory info, if available.

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

[dpapad, 2016-05-24 18:28:25]

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

[dpapad, 2016-05-24 18:28:25]

Screenshot at http://imgur.com/NZUMPES.

[dschuyler, 2016-05-24 20:21:41]

In the screen shot, it looks like the left-most
vertical pixel column is cut off.

https://codereview.chromium.org/2006073002/diff/20001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/about_page_tests.js (right):

https://codereview.chromium.org/2006073002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/about_page_tests.js:123: /** @override */
Should this have a @return?

https://codereview.chromium.org/2006073002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/about_page_tests.js:291: *     visible.
@return?

[dpapad, 2016-05-24 21:20:18]

The CQ bit was checked by dpapad@chromium.org

[dpapad, 2016-05-24 21:20:20]

The CQ bit was unchecked by dpapad@chromium.org

[dpapad, 2016-05-24 21:21:08]

The left-most vertical pixel column is missing from the image itself, is not
being cut off. You can verify this by going to chrome://help on your Chromebook.

https://codereview.chromium.org/2006073002/diff/20001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/about_page_tests.js (right):

https://codereview.chromium.org/2006073002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/about_page_tests.js:123: /** @override */
On 2016/05/24 at 20:21:41, dschuyler wrote:
> Should this have a @return?

Why is this different than any other method that has @override? The overriden
method has @return specified so no @return should be needed here.

https://codereview.chromium.org/2006073002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/about_page_tests.js:291: *     visible.
On 2016/05/24 at 20:21:41, dschuyler wrote:
> @return?

Done.

[dpapad, 2016-05-24 21:22:29]

On 2016/05/24 at 21:21:08, dpapad wrote:
> The left-most vertical pixel column is missing from the image itself, is not
being cut off. You can verify this by going to chrome://help on your Chromebook.

Or you can visit http://i.imgur.com/GEQX6Oe.png if you don't have a Chromebook
around.

> 
>
https://codereview.chromium.org/2006073002/diff/20001/chrome/test/data/webui/...
> File chrome/test/data/webui/settings/about_page_tests.js (right):
> 
>
https://codereview.chromium.org/2006073002/diff/20001/chrome/test/data/webui/...
> chrome/test/data/webui/settings/about_page_tests.js:123: /** @override */
> On 2016/05/24 at 20:21:41, dschuyler wrote:
> > Should this have a @return?
> 
> Why is this different than any other method that has @override? The overriden
method has @return specified so no @return should be needed here.
> 
>
https://codereview.chromium.org/2006073002/diff/20001/chrome/test/data/webui/...
> chrome/test/data/webui/settings/about_page_tests.js:291: *     visible.
> On 2016/05/24 at 20:21:41, dschuyler wrote:
> > @return?
> 
> Done.

[dschuyler, 2016-05-24 21:28:37]

lgtm

[dpapad, 2016-05-24 21:34:17]

On 2016/05/24 at 21:28:37, dschuyler wrote:
> lgtm

Thanks!

[dpapad, 2016-05-24 21:34:23]

The CQ bit was checked by dpapad@chromium.org

[commit-bot: I haz the power, 2016-05-24 21:34:47]

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

[commit-bot: I haz the power, 2016-05-24 23:19:36]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-24 23:20:26]

Description was changed from

==========
MD Settings: About page, show regulatory info, if available.

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

to

==========
MD Settings: About page, show regulatory info, if available.

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

Committed: https://crrev.com/b8c65611ac43f8d95f2f46f91d80c0284589b64f
Cr-Commit-Position: refs/heads/master@{#395726}
==========

[commit-bot: I haz the power, 2016-05-24 23:20:27]

Patchset 3 (id:??) landed as
https://crrev.com/b8c65611ac43f8d95f2f46f91d80c0284589b64f
Cr-Commit-Position: refs/heads/master@{#395726}

[michaelpg, 2016-06-24 07:20:34]

On 2016/05/24 18:28:25, dpapad wrote:
> Screenshot at http://imgur.com/NZUMPES.

how did you get this screenshot? from what I can tell the label image filename
has always been broken (https://codereview.chromium.org/1971483002)

[dpapad, 2016-06-24 16:23:09]

On 2016/06/24 at 07:20:34, michaelpg wrote:
> On 2016/05/24 18:28:25, dpapad wrote:
> > Screenshot at http://imgur.com/NZUMPES.
> 
> how did you get this screenshot? from what I can tell the label image filename
has always been broken (https://codereview.chromium.org/1971483002)

I don't remember doing anything special to get the screenshot. This worked the
same way the old code works at
https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/help/help_handle...,
AFAIU.

[dpapad, 2016-06-24 16:31:22]

On 2016/06/24 at 16:23:09, dpapad wrote:
> On 2016/06/24 at 07:20:34, michaelpg wrote:
> > On 2016/05/24 18:28:25, dpapad wrote:
> > > Screenshot at http://imgur.com/NZUMPES.
> > 
> > how did you get this screenshot? from what I can tell the label image
filename has always been broken (https://codereview.chromium.org/1971483002)
> 
> I don't remember doing anything special to get the screenshot. This worked the
same way the old code works at
https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/help/help_handle...,
AFAIU.

Now that I saw the bug at https://codereview.chromium.org/2094703005, I guess I
must have modified the DOM manually to produce this screenshot. I certainly did
not know the trick of putting an image at
/usr/share/chromeos-assets/regulatory_labels/us/ folder to trigger the label to
be shown, which would explain why I did not catch this bug. Thanks for finding
and fixing.

[michaelpg, 2016-06-24 18:18:27]

On 2016/06/24 16:31:22, dpapad wrote:
> On 2016/06/24 at 16:23:09, dpapad wrote:
> > On 2016/06/24 at 07:20:34, michaelpg wrote:
> > > On 2016/05/24 18:28:25, dpapad wrote:
> > > > Screenshot at http://imgur.com/NZUMPES.
> > > 
> > > how did you get this screenshot? from what I can tell the label image
> filename has always been broken (https://codereview.chromium.org/1971483002)
> > 
> > I don't remember doing anything special to get the screenshot. This worked
the
> same way the old code works at
>
https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/help/help_handle...,
> AFAIU.
> 
> Now that I saw the bug at https://codereview.chromium.org/2094703005, I guess
I
> must have modified the DOM manually to produce this screenshot. I certainly
did
> not know the trick of putting an image at
> /usr/share/chromeos-assets/regulatory_labels/us/ folder to trigger the label
to
> be shown, which would explain why I did not catch this bug. Thanks for finding
> and fixing.

No problem. Hopefully the new "label_dir_path" name will make this clearer for
whoever works on the next rewrite :-)