[PageAction] Add support for chrome.pageAction.hide on Android

RESOLVED FIXED in Firefox 49

Status

()

Toolkit
WebExtensions: Untriaged
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mattw, Assigned: mattw)

Tracking

(Blocks: 1 bug)

unspecified
mozilla49
Unspecified
Android
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

(Whiteboard: [pageAction]triaged)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
This bug is meant to track the implementation and testing of Chrome.pageAction.hide on android.

Firefox docs: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/PageAction/hide
Chrome docs: https://developer.chrome.com/extensions/pageAction#method-hide

Notes: We should be able to implement this method by wrapping `PageActions.hide` in the existing PageActions.jsm module.
(Assignee)

Updated

2 years ago
Summary: [PageAction] Add support for chrome.pageAction.hide → [PageAction] Add support for chrome.pageAction.hide on Android
(Assignee)

Updated

2 years ago
Whiteboard: triaged → [PageAction]triaged
(Assignee)

Updated

2 years ago
Whiteboard: [PageAction]triaged → [pageAction]triaged
(Assignee)

Updated

2 years ago
Assignee: nobody → mwein
(Assignee)

Comment 1

2 years ago
Created attachment 8752389 [details]
MozReview Request: Bug 1267346 - Implement chrome.PageAction.hide on Android r?kmag

Review commit: https://reviewboard.mozilla.org/r/52581/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52581/
Attachment #8752389 - Flags: review?(kmaglione+bmo)
Comment on attachment 8752389 [details]
MozReview Request: Bug 1267346 - Implement chrome.PageAction.hide on Android r?kmag

https://reviewboard.mozilla.org/r/52581/#review49570

::: mobile/android/components/extensions/ext-pageAction.js:42
(Diff revision 1)
> +  },
> +
>    shutdown() {
>      if (this.id) {
>        PageActions.remove(this.id);
>        this.id = null;

We should probably just call `this.hide()` here, now.

::: mobile/android/components/extensions/test/mochitest/test_ext_pageAction.html:30
(Diff revision 1)
> -  browser.test.sendMessage("page-action-shown");
> +      browser.test.sendMessage("page-action-shown");
> +    } else if (msg === "pageAction-hide") {
> +      browser.pageAction.hide(tabId);
> +      browser.test.sendMessage("page-action-hidden");
> +    } else if (msg === "pageAction-finish") {
> +      browser.test.neotifyPass("page-action");

*notifyPass

::: mobile/android/components/extensions/test/mochitest/test_ext_pageAction.html:53
(Diff revision 1)
>    yield extension.startup();
> +  yield extension.awaitMessage("ready");
> +
> +  extension.sendMessage("pageAction-show");
>    yield extension.awaitMessage("page-action-shown");
> +  is(isPageActionShown(extension.id), true, "The PageAction should be shown");

`ok(isPageActionShown(...), ...)`

::: mobile/android/components/extensions/test/mochitest/test_ext_pageAction.html:57
(Diff revision 1)
>    yield extension.awaitMessage("page-action-shown");
> +  is(isPageActionShown(extension.id), true, "The PageAction should be shown");
> +
> +  extension.sendMessage("pageAction-hide");
> +  yield extension.awaitMessage("page-action-hidden");
> +  is(isPageActionShown(extension.id), false, "The PageAction should be hidden");

`ok(!isPageActionShown(...), ...)`

::: mobile/android/components/extensions/test/mochitest/test_ext_pageAction.html:63
(Diff revision 1)
>  
> +  extension.sendMessage("pageAction-show");
> +  yield extension.awaitMessage("page-action-shown");
>    is(isPageActionShown(extension.id), true, "The PageAction should be shown");
>  
> +  extension.sendMessage("pageAction-finish");

Is this really necessary?
Attachment #8752389 - Flags: review?(kmaglione+bmo) → review+
(Assignee)

Comment 3

2 years ago
Comment on attachment 8752389 [details]
MozReview Request: Bug 1267346 - Implement chrome.PageAction.hide on Android r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52581/diff/1-2/
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 4

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b028268f4e20
Keywords: checkin-needed

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b028268f4e20
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.