Closed Bug 1375857 Opened 3 years ago Closed 3 years ago

WebExtension is displayed in the main menu drop-down list after it was uninstalled (on android)

Categories

(WebExtensions :: Android, defect, P1)

56 Branch
Unspecified
Android
defect

Tracking

(firefox55 unaffected, firefox56 verified, firefox57 verified)

VERIFIED FIXED
mozilla57
Tracking Status
firefox55 --- unaffected
firefox56 --- verified
firefox57 --- verified

People

(Reporter: cbadescu, Assigned: rpl)

References

Details

Attachments

(4 files)

Attached image BugZ.gif
[Affected versions]:
- Firefox 56.0a1 (2017-06-23) 
 
[Affected platforms]:
- Android 7.1.2
 
[Steps to reproduce]:
1.Install https://adons-dev.allizom.org/en-US/android/addon/bookmark-it/ 
2.From the main menu drop-down list select tap on the installed WebExtension.
3.Uninstall the WebExtension.
4.Open main menu again and scroll down to the installed WebExtension.
 
[Expected results]:
- The WebExtesions is not displayed in the main menu drop-down list after it was uninstalled.
 
[Actual results]:
- The WebExtesions is displayed in the main menu drop-down list after it was uninstalled.
 
[Additional notes]:
- This issue reproduces intermittently.
- Please see the attached video for this issue.
I couldn't check if this issue reproduces on Firefox Beta 55 because I am blocked by this Bug 1367494.
Blocks: 1330159
Summary: WebExtesion is displayed in the main menu drop-down list after it was uninstalled → WebExtesion is displayed in the main menu drop-down list after it was uninstalled (on android)
Hey Matt, can you please check this out?
Flags: needinfo?(mwein)
I'd like to try reproducing this but I'm currently unable to install the add-on at https://addons-dev.allizom.org/en-US/android/addon/bookmark-it/ because it appears to be corrupt - is there anything special I need to do in addition to enabling xpinstall.signatures.required in order to download it?
Flags: needinfo?(mwein)
Based on attached video, it looks like something wrong is happening with the "Report site issue" button..
These are the settings that I use in order to install that add-on:

xpinstall.signatures.dev-root - set to true
extensions.webapi.testing - set to true
extensions.webservice.discoverURL - https://discovery.addons-dev.allizom.org/

Sorry for not mentioning this, Matthew.
Assignee: nobody → lgreco
Status: NEW → ASSIGNED
Priority: -- → P1
I've been able to reproduce this issue and dig into it a bit:

it looks like that the browserAction menu items are corrupting the cache of the addon menu items (and I guess that the "Report site issue" is defined as an addon menu item), which becomes evident when the webextension is disabled or uninstalled and the menu is regenerated (and at that point the "Report site issue" menu item is updated using the title of the removed browserAction).

I've also attached to this issue the change that I've applied locally to fix the issue.
Attachment #8891020 - Flags: review?(s.kaspari)
Comment on attachment 8891020 [details]
Bug 1375857 - Fix browserAction Android menu items corrupting addon menu items cache.

https://reviewboard.mozilla.org/r/162206/#review168752
Attachment #8891020 - Flags: review?(s.kaspari) → review+
Attachment #8891020 - Flags: review?(matthewjwein)
Summary: WebExtesion is displayed in the main menu drop-down list after it was uninstalled (on android) → WebExtension is displayed in the main menu drop-down list after it was uninstalled (on android)
Attachment #8891020 - Flags: review?(matthewjwein)
Pushed by luca.greco@alcacoop.it:
https://hg.mozilla.org/integration/autoland/rev/5d0a3f673688
Fix browserAction Android menu items corrupting addon menu items cache. r=sebastian
https://hg.mozilla.org/mozilla-central/rev/5d0a3f673688
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Attached image Animation.gif
This issue is verified as fixed on Fennec 57.0a1 (2017-08-08) under Android 7.1.2. The webextension menu item is no longer displayed after the webextension was uninstalled. See attached screencast.
Status: RESOLVED → VERIFIED
Comment on attachment 8891020 [details]
Bug 1375857 - Fix browserAction Android menu items corrupting addon menu items cache.

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1331742 - Add support for browserAction.onClicked
and
Bug 1351111 - Add support for browserAction.setTitle and browserAction.getTitle

which  introduced the new browserActions.jsm helper and a related set of changes in the BrowserApp Java class.

[User impact if declined]:
If a user installs and then uninstalls a WebExtension add-on which contains a browserAction, then the menu items created with the internal menu items API
(e.g. like the "Report Site" menu item integrated in Fennec, or menu items
defined by other addons that use the internal menu items API)
will be corrupted by the label of the browserAction 
(which will be very confusing for the user, e.g. the add-on would seem to
has not been fully uninstalled, and clicking the button will actually have a unexpected behavior because the corrupted menu items will be executed instead of the action expected by the related browserAction).

[Is this code covered by automated tests?]:
No

[Has the fix been verified in Nightly?]:
Yes

[Needs manual test from QE? If yes, steps to reproduce]: 
Yes, the same STR used to verify the fix on Nightly should be used to verify it
on beta.

[List of other uplifts needed for the feature/fix]:
- Bug 1387026 - Fix Android browserAction corrupting legacy Addon menu items label on overlapping menu item id

fixes a similar issue which happens when more than one WebExtension add-on with a browserAction are installed and the menu ids assigned by the browserAction and the ones assigned by the internal menu items API are going to overlap and an update of the browserAction title can update the wrong menu item because they use the same id. 

[Is the change risky?]:
Low

[Why is the change risky/not risky?]:
Bug 1375857 contains a very small change on the Java bits related to this feature (1 line change which fixes a typo), it only impact on the browserAction Java internals and none of the other features should be impacted by it.

[String changes made/needed]:
None
Attachment #8891020 - Flags: approval-mozilla-beta?
Comment on attachment 8891020 [details]
Bug 1375857 - Fix browserAction Android menu items corrupting addon menu items cache.

Should get FxA menu items working again for beta 3.
Attachment #8891020 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Duplicate of this bug: 1390090
Duplicate of this bug: 1390099
Can this patch be uplifted to Firefox 55 too?

Arguments for uplifting:
- Firefox 55 is the first version where browserAction.onClicked is supported (bug 1331742).

- The browserAction menu item is a very visible UI element, and merely opening the menu is a sufficient condition for any extension-added menu item to not respond to tapping (specific examples include bug 1390090 and bug 1390099).

- Even the most obvious scenario leads to a seemingly disfunctional add-on:
1. Open the menu and choose "Addons" (the act of opening the menu triggers the bug).
2. Search for "[some name of addon that someone wants to demo]" and install it.
3. Open the menu and choose "[some menu item name]" to use the add-on.

- Add-ons have no way of detecting that this bug occurs, so they cannot try to fall back gracefully.

- Firefox 55 was released for Android just last week, so I expect another release (to ride along with) to be likely.

Arguments against uplifting:
- Menu items are registered when an add-on starts up. If the Firefox app is restarted, the add-on starts too. Menu items for previously installed add-ons will often work, because the user will probably not try to hit the menu in the first seconds of Firefox startup (=bug 1390099).
- Firefox for Android only claims support for WebExtensions in Firefox 56 (where this bug has been fixed) - https://www.mozilla.org/en-US/firefox/android/56.0beta/releasenotes/
Attached image FixMenu.gif
This issue is verified as fixed on Fennec 56.0b3 under Android 6.0.1. 

The webextension menu item is no longer displayed after the webextension was uninstalled.
See Also: → 1427255
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.