Closed
Bug 1384964
Opened 7 years ago
Closed 7 years ago
pageAction show and hide do not work on tabId 0 on Android
Categories
(WebExtensions :: Android, defect, P1)
WebExtensions
Android
Tracking
(firefox56 verified, firefox57 verified)
VERIFIED
FIXED
mozilla56
People
(Reporter: rpl, Assigned: rpl)
References
Details
Attachments
(4 files)
The pageAction show and hide implementation on Firefox for Android is currently unable to work on tabId 0, because of the following ternary `tabId ? tabTracker.getTab(tabId) : null;` which will return null for `tabId == 0`:
- http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/mobile/android/components/extensions/ext-pageAction.js#240,245
(also getPopup and setPopup seems to have the same issue: http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/mobile/android/components/extensions/ext-pageAction.js#250,256)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8890921 -
Flags: review?(mixedpuppy)
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Comment 2•7 years ago
|
||
There should be no real reason to check the tabId manually inside this API methods, the type of this parameter is already checked by the schema wrappers.
I would have liked to also change the test cases to explicitly test to guard this from regressing again, unfortunately the tabId 0 is used by the test harness in the mochitests (that is likely to be the reason why it wasn't tested with the tabId 0), and so I opted to attach a fix for the issue as soon as possible.
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8890921 [details]
Bug 1384964 - Fix pageAction.show/hide/getPopup/setPopup on Android for tabId 0.
https://reviewboard.mozilla.org/r/162124/#review167436
Attachment #8890921 -
Flags: review?(mixedpuppy) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•7 years ago
|
||
I noticed that browserAction.setTitle/getTitle have a similar issue with tabId 0 (the main difference is that browserAction tabId parameter is actually optional and so we need to keep the ternary but fix it to detect tabId 0 as a valid tabId).
Comment hidden (mozreview-request) |
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8891347 [details]
Bug 1384964 - Fix browserAction.setTitle/getTitle on Android for tabId 0.
https://reviewboard.mozilla.org/r/162544/#review167878
Attachment #8891347 -
Flags: review?(mixedpuppy) → review+
Pushed by luca.greco@alcacoop.it:
https://hg.mozilla.org/integration/autoland/rev/60076d86bb5c
Fix pageAction.show/hide/getPopup/setPopup on Android for tabId 0. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/5c6a8f30184c
Fix browserAction.setTitle/getTitle on Android for tabId 0. r=mixedpuppy
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/60076d86bb5c
https://hg.mozilla.org/mozilla-central/rev/5c6a8f30184c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment 11•7 years ago
|
||
This issue is verified as fixed on Fennec 57.0a1 (2017-08-28) under Android 6.0.1.
The pageAction.show/hide, browserAction setTitle/getTitle, pageAction setPopup/getPopup are working on the web page that has tabId 0.
(On Fennec 56.0b5 under Android 6.0.1 browserAction setTitle/getTitle is not working because of the Bug 1392322 which was fixed only in Fennec 57, the pageAction.show/hide and pageAction setPopup/getPopup work as expected in Fennec 56.0b5.)
Please see the attached video.
Status: RESOLVED → VERIFIED
status-firefox57:
--- → verified
Comment 12•7 years ago
|
||
This issue is verified as fixed on Fennec 56.0b7 under Android 6.0.1.
With the uplift of Bug 1392322 on Fennec 56.0b7,the pageAction.show/hide, browserAction setTitle/getTitle, pageAction setPopup/getPopup are working on the web page that has tabId 0.
Please see the attached video.
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•