Reorder "action links" on chrome://extensions

Reorder "action links" on chrome://extensions

Move the Options, Permissions, Visit website, Launch, and
Reload links to be on the same line in the
chrome://extensions page, directly below the description (so
they are in the same place for all extensions in the list).

Also reword "Visit website" to be either "Details" or
"Developer website", depending on whether it will link to
the developer's website or the chrome web store.

Finally, tweak the css to give a bit more padding to the
description and make the version text grey.

BUG=410895

[Devlin, 2014-09-04 17:52:57]

Patchset #1 (id:1) has been deleted

[Devlin, 2014-09-04 18:28:33]

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

[Devlin, 2014-09-04 18:28:34]

Hey Dan, mind taking a look?

[Dan Beam, 2014-09-04 23:01:20]

https://codereview.chromium.org/540113002/diff/40001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (left):

https://codereview.chromium.org/540113002/diff/40001/chrome/app/generated_res...
chrome/app/generated_resources.grd:4800: <message
name="IDS_EXTENSIONS_VISIT_WEBSTORE" desc="The link for visiting the extension's
gallery page.">
do these descriptions need to be updated?

https://codereview.chromium.org/540113002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/540113002/diff/40001/chrome/browser/resources...
chrome/browser/resources/extensions/extension_list.js:204:
document.createElement('span')).textContent = extension.description;
can you make this code less sly by adding temporary variables?

https://codereview.chromium.org/540113002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/extensions/extensions.css (right):

https://codereview.chromium.org/540113002/diff/40001/chrome/browser/resources...
chrome/browser/resources/extensions/extensions.css:227: .action-links > a {
why do you need > ?

https://codereview.chromium.org/540113002/diff/40001/chrome/browser/resources...
chrome/browser/resources/extensions/extensions.css:227: .action-links > a {
if not having needless margin on the end matter, you probably want:

  .action-links a:not(:last-of-type) {
    -webkit-margin-end: 1em;
  }

if they're all siblings.

https://codereview.chromium.org/540113002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/extensions/extensions.html (right):

https://codereview.chromium.org/540113002/diff/40001/chrome/browser/resources...
chrome/browser/resources/extensions/extensions.html:139: href="#" hidden></a>
why is this an <a> if the [href] isn't useful?  why not .link-button?

https://codereview.chromium.org/540113002/diff/40001/chrome/common/extensions...
File chrome/common/extensions/manifest_url_handler.h (right):

https://codereview.chromium.org/540113002/diff/40001/chrome/common/extensions...
chrome/common/extensions/manifest_url_handler.h:35: static bool
SpecifiedHomepageURL(const Extension* extension);
why aren't these const&?

[Devlin, 2014-09-04 23:28:10]

https://codereview.chromium.org/540113002/diff/40001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (left):

https://codereview.chromium.org/540113002/diff/40001/chrome/app/generated_res...
chrome/app/generated_resources.grd:4800: <message
name="IDS_EXTENSIONS_VISIT_WEBSTORE" desc="The link for visiting the extension's
gallery page.">
On 2014/09/04 23:01:19, Dan Beam wrote:
> do these descriptions need to be updated?

Surprisingly, no.  The descriptions were correct, and the intent was correct -
we just weren't determining if it was an extension's homepage vs the gallery
page properly before.

https://codereview.chromium.org/540113002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/540113002/diff/40001/chrome/browser/resources...
chrome/browser/resources/extensions/extension_list.js:204:
document.createElement('span')).textContent = extension.description;
On 2014/09/04 23:01:19, Dan Beam wrote:
> can you make this code less sly by adding temporary variables?

Done. Lemme know if you want more/less.

https://codereview.chromium.org/540113002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/extensions/extensions.css (right):

https://codereview.chromium.org/540113002/diff/40001/chrome/browser/resources...
chrome/browser/resources/extensions/extensions.css:227: .action-links > a {
On 2014/09/04 23:01:19, Dan Beam wrote:
> why do you need > ?

Don't need.  Just like to be specific, and right now, all the a's we care about
are immediate children.  I'll take it out, though.

Update: Slight change to accommodate link buttons, lemme know if it should be
tweaked again.

https://codereview.chromium.org/540113002/diff/40001/chrome/browser/resources...
chrome/browser/resources/extensions/extensions.css:227: .action-links > a {
On 2014/09/04 23:01:19, Dan Beam wrote:
> if not having needless margin on the end matter, you probably want:
> 
>   .action-links a:not(:last-of-type) {
>     -webkit-margin-end: 1em;
>   }
> 
> if they're all siblings.

Couple of things:
- It shouldn't matter, so I'd be inclined not to put it in (it's kind of a
mouthful).
- not(:last-of-type) doesn't really work, because the last of the type (the
reload link) isn't always the last *visible* of type, so there could still be
trailing margins.  I guess if we wanted to get really fancy, we could instead
target the first visible child using the semi-hack of "any child that's visible
and has no visible parent", but that seems like overkill.

https://codereview.chromium.org/540113002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/extensions/extensions.html (right):

https://codereview.chromium.org/540113002/diff/40001/chrome/browser/resources...
chrome/browser/resources/extensions/extensions.html:139: href="#" hidden></a>
On 2014/09/04 23:01:19, Dan Beam wrote:
> why is this an <a> if the [href] isn't useful?  why not .link-button?

For that most terrible of reasons: I copied the code.  Fixed them to make it
better. :)

Note, however, this results in the good/bad effect of only having the link
underline for the details/developer website links.  This is
Good: It keeps in line with what we do everywhere else, saying "underline if it
means navigate away", otherwise don't."
Bad:  It looks kinda weird to have one "randomly" underlined link in the row.

Until told otherwise (possibly by UI team), I'm going to assume the good
outweighs the bad in this case.

https://codereview.chromium.org/540113002/diff/40001/chrome/common/extensions...
File chrome/common/extensions/manifest_url_handler.h (right):

https://codereview.chromium.org/540113002/diff/40001/chrome/common/extensions...
chrome/common/extensions/manifest_url_handler.h:35: static bool
SpecifiedHomepageURL(const Extension* extension);
On 2014/09/04 23:01:19, Dan Beam wrote:
> why aren't these const&?

For whatever reason, almost all extensions code uses const Extension* instead of
const Extension&.  I don't think anyone really knows why, and now we do it for
conformance reasons.

[Dan Beam, 2014-09-05 00:09:35]

lgtm

https://codereview.chromium.org/540113002/diff/40001/chrome/common/extensions...
File chrome/common/extensions/manifest_url_handler.h (right):

https://codereview.chromium.org/540113002/diff/40001/chrome/common/extensions...
chrome/common/extensions/manifest_url_handler.h:35: static bool
SpecifiedHomepageURL(const Extension* extension);
On 2014/09/04 23:28:10, Devlin wrote:
> On 2014/09/04 23:01:19, Dan Beam wrote:
> > why aren't these const&?
> 
> For whatever reason, almost all extensions code uses const Extension* instead
of
> const Extension&.  I don't think anyone really knows why, and now we do it for
> conformance reasons.

eh, it's probably to avoid creating more pointers/references

https://codereview.chromium.org/540113002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/extensions/extensions.css (right):

https://codereview.chromium.org/540113002/diff/60001/chrome/browser/resources...
chrome/browser/resources/extensions/extensions.css:228: .action-links a {
nit: for extra fancy points:

  .action-links :-webkit-any(a, .link-button) {

[Devlin, 2014-09-05 00:16:37]

https://codereview.chromium.org/540113002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/extensions/extensions.css (right):

https://codereview.chromium.org/540113002/diff/60001/chrome/browser/resources...
chrome/browser/resources/extensions/extensions.css:228: .action-links a {
On 2014/09/05 00:09:34, Dan Beam wrote:
> nit: for extra fancy points:
> 
>   .action-links :-webkit-any(a, .link-button) {

Nifty. Done.

[Devlin, 2014-09-05 00:17:16]

rdevlin.cronin@chromium.org changed reviewers:
+ yoz@chromium.org

[Devlin, 2014-09-05 00:17:17]

Yoyo, mind taking a look at manifest_url_handler real quick?

[Yoyo Zhou, 2014-09-05 00:21:36]

On 2014/09/05 00:17:17, Devlin wrote:
> Yoyo, mind taking a look at manifest_url_handler real quick?

it LGTM

[Devlin, 2014-09-05 21:49:35]

PSA: This patch has been moved to https://codereview.chromium.org/550443003/
because I accidentally uploaded this one tracking a local branch, which messes
up the Base URL here on Rietveld.

Apologies for the noise.