Add browserAction.enable/disable as alias for pageAction.show/hide.

Add browserAction.enable/disable as alias for pageAction.show/hide.

Depends on http://codereview.chromium.org/10700007/

BUG=133094
TEST=Use browserAction.disable/enable(tabid), observe that the browser action becomes grayed out.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=148043

[Yoyo Zhou, 2012-07-17 00:01:46]

P.S. Not sure why there are a bunch of unrelated docs changes; someone hasn't
been generating them, I suppose.

[not at google - send to devlin, 2012-07-17 13:18:02]

The tab ID in the methods *should* probably be optional; it's consistent with
the rest of the browser action API. It would involve just a teeny bit of extra
work...

And yeah, stupid docs. Can't wait for the new server.

http://codereview.chromium.org/10789026/diff/1/chrome/common/extensions/api/b...
File chrome/common/extensions/api/browser_action.json (right):

http://codereview.chromium.org/10789026/diff/1/chrome/common/extensions/api/b...
chrome/common/extensions/api/browser_action.json:268: {"type": "integer",
"name": "tabId", "minimum": 0, "description": "The id of the tab for which you
want to modify the browser action."}
should this be optional to disable for all tabs?

http://codereview.chromium.org/10789026/diff/1/chrome/common/extensions/api/b...
chrome/common/extensions/api/browser_action.json:276: {"type": "integer",
"name": "tabId", "minimum": 0, "description": "The id of the tab for which you
want to modify the browser action."}
ditto

[Yoyo Zhou, 2012-07-20 21:23:41]

I made the tabid argument optional. Turns out it only required a small change to
extension_actions_api.cc.

[not at google - send to devlin, 2012-07-23 11:07:47]

lgtm but you might want to change that tabId thing. +jyasskin about that.

http://codereview.chromium.org/10789026/diff/5001/chrome/common/extensions/ap...
File chrome/common/extensions/api/browser_action.json (right):

http://codereview.chromium.org/10789026/diff/5001/chrome/common/extensions/ap...
chrome/common/extensions/api/browser_action.json:271: "name": "tabId",
Sorry I just noticed this (and I reviewed the change that added it, ah well):
why is scriptBadge.show/hide (and therefore browserAction.enable/disable)
different to all other extension action functions? Those take a "details" object
(often with only a single property "tabId") but this just takes a single integer
for tabId.

For consistency, we might want to change that (and perhaps also consider making
these functions that accept just a single "tabId" property to accept an integer
too, like, using "choices" in the schema).

[Yoyo Zhou, 2012-07-23 14:25:16]

http://codereview.chromium.org/10789026/diff/5001/chrome/common/extensions/ap...
File chrome/common/extensions/api/browser_action.json (right):

http://codereview.chromium.org/10789026/diff/5001/chrome/common/extensions/ap...
chrome/common/extensions/api/browser_action.json:271: "name": "tabId",
On 2012/07/23 11:07:48, kalman wrote:
> Sorry I just noticed this (and I reviewed the change that added it, ah well):
> why is scriptBadge.show/hide (and therefore browserAction.enable/disable)
> different to all other extension action functions? Those take a "details"
object
> (often with only a single property "tabId") but this just takes a single
integer
> for tabId.

I believe it's for consistency with pageAction.show/hide -- the other thing they
have in common is no callback.

> For consistency, we might want to change that (and perhaps also consider
making
> these functions that accept just a single "tabId" property to accept an
integer
> too, like, using "choices" in the schema).

You could argue that getX should aim for consistency with setX. In my opinion it
would be most consistent if they all took a details object, but since
pageAction.show/hide don't, I'm reluctant to change it.

[not at google - send to devlin, 2012-07-23 14:27:13]

Cool

http://codereview.chromium.org/10789026/diff/5001/chrome/common/extensions/ap...
File chrome/common/extensions/api/browser_action.json (right):

http://codereview.chromium.org/10789026/diff/5001/chrome/common/extensions/ap...
chrome/common/extensions/api/browser_action.json:271: "name": "tabId",
On 2012/07/23 14:25:16, Yoyo Zhou wrote:
> On 2012/07/23 11:07:48, kalman wrote:
> > Sorry I just noticed this (and I reviewed the change that added it, ah
well):
> > why is scriptBadge.show/hide (and therefore browserAction.enable/disable)
> > different to all other extension action functions? Those take a "details"
> object
> > (often with only a single property "tabId") but this just takes a single
> integer
> > for tabId.
> 
> I believe it's for consistency with pageAction.show/hide -- the other thing
they
> have in common is no callback.
> 
> > For consistency, we might want to change that (and perhaps also consider
> making
> > these functions that accept just a single "tabId" property to accept an
> integer
> > too, like, using "choices" in the schema).
> 
> You could argue that getX should aim for consistency with setX. In my opinion
it
> would be most consistent if they all took a details object, but since
> pageAction.show/hide don't, I'm reluctant to change it.

Right you are.

[Jeffrey Yasskin, 2012-07-23 17:00:02]

http://codereview.chromium.org/10789026/diff/5001/chrome/common/extensions/ap...
File chrome/common/extensions/api/browser_action.json (right):

http://codereview.chromium.org/10789026/diff/5001/chrome/common/extensions/ap...
chrome/common/extensions/api/browser_action.json:271: "name": "tabId",
On 2012/07/23 11:07:48, kalman wrote:
> Sorry I just noticed this (and I reviewed the change that added it, ah well):
> why is scriptBadge.show/hide (and therefore browserAction.enable/disable)
> different to all other extension action functions? Those take a "details"
object
> (often with only a single property "tabId") but this just takes a single
integer
> for tabId.
> 
> For consistency, we might want to change that (and perhaps also consider
making
> these functions that accept just a single "tabId" property to accept an
integer
> too, like, using "choices" in the schema).

If it's really _just_ show/hide that are different in this way, I think it'd
make sense to change them to be more consistent. People will eventually stop
writing new page actions, so keeping consistency with them doesn't seem as
important as consistency across the vNext API.

If y'all agree, I'll write a change for this.

[Yoyo Zhou, 2012-07-23 23:11:12]

Is there really a scriptBadge.show? I don't see one in the json spec, and I
don't think it would actually make a lot of sense.

I do buy the argument that in our brave new world there are no such things as
page actions, so it's fine to have browserAction.enable/disable be consistent
with the browserAction API and not consistent with the deprecated pageAction
API.

[Jeffrey Yasskin, 2012-07-24 00:00:20]

On Mon, Jul 23, 2012 at 4:11 PM,  <yoz@chromium.org> wrote:
> Is there really a scriptBadge.show? I don't see one in the json spec, and I
> don't think it would actually make a lot of sense.

Don't assume I know what I'm talking about. ;)

> I do buy the argument that in our brave new world there are no such things
> as
> page actions, so it's fine to have browserAction.enable/disable be
> consistent
> with the browserAction API and not consistent with the deprecated pageAction
> API.
>
> http://codereview.chromium.org/10789026/

[Yoyo Zhou, 2012-07-24 01:21:50]

I've decided to just leave it as is. No details, just the integer tabId
argument.

[commit-bot: I haz the power, 2012-07-24 01:23:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoz@chromium.org/10789026/7008

[commit-bot: I haz the power, 2012-07-24 03:42:26]

Change committed as 148043