Menu items added by add-ons disappear if "Don't keep activities" is checked

RESOLVED FIXED in Firefox 59

Status

()

RESOLVED FIXED
6 years ago
5 months ago

People

(Reporter: kats, Assigned: JanH)

Tracking

Trunk
Firefox 60
All
Android
Points:
---

Firefox Tracking Flags

(fennec+, firefox57 wontfix, firefox58 wontfix, firefox59 verified, firefox60 verified)

Details

(Whiteboard: [add-ons])

MozReview Requests

()

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

Attachments

(3 attachments)

STR:
- Turn on the "don't keep activities" checkbox in the developer settings
- Install an add-on that has a menu item (I used capella's QuitNow addon)
- Start Fennec, ensure that the menu item shows up
- Open the awesomebar screen and load a page (you may have to do this twice, for some reason it didn't happen the first time for me)
- Check the menu to see if the QuitNow menu item is still there

Actual:
- It is not.

It looks like we keep the menu items in the mAddonMenuItemsCache until the first time we build the menu, and then clear that list. If the activity goes away, so do the menu items. We should probably keep that list around.
I think this is still an issue that add-on authors are running into. See for example [1] which says:

"NOT WORKING? There is a known Firefox bug that affects this and other add-ons.

If on your Android device, under Settings -> Developer Options, you have "Do not keep activities" selected / checked, Firefox will lose add-on menu items ... Uncheck that option, to solve that problem :-)"

[1] https://addons.mozilla.org/en-US/android/addon/cleanquit/
tracking-fennec: --- → ?
I added code to fix a similar situation, triggered by the locale switcher.

https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java#2440

The trick with this bug is that we're persisting the menu items in BrowserApp itself, which is being thrown away each time. If we fix that -- persisting the menu items list outside of BrowserApp -- then everything else should work correctly.

Comment 3

3 years ago
We should fix this to make sure the menu add-on API works reliably, but it's not high priority for us right now.
tracking-fennec: ? → +
Whiteboard: [add-ons]

Comment 4

10 months ago
This happens to me even if I don't have "don't keep activities" checkbox in the developer settings selected. I disabled developer settings completly.

I can reproduce easily by:

- open Firefox, and check if menu entries are present (I have three, for uBlock, Cookie AutoDelete and Violentmonkey)
- go to system settings -> display -> font size
- change font size (medium to small or back)
- go back to Firefox
- menu entries are missing

This is on Android 4.4.2 with Firefox Nightly 58.0a1 (2017-10-22)
(Assignee)

Comment 5

10 months ago
(In reply to gwarser from comment #4)
> This happens to me even if I don't have "don't keep activities" checkbox in
> the developer settings selected.

Yes, that option is intended to force this behaviour every time the app is backgrounded to make testing easier, but when resources are running low it can happen for real even without that option being active.
(Assignee)

Updated

9 months ago
See Also: → bug 1396270
(Assignee)

Updated

9 months ago
Duplicate of this bug: 1419062
(Assignee)

Updated

9 months ago
tracking-fennec: + → ?
status-firefox57: --- → affected
status-firefox58: --- → affected
status-firefox59: --- → affected
Version: unspecified → Trunk
Hi Joe, Wesly
Please help prioritize this.
Flags: needinfo?(wehuang)
Flags: needinfo?(jcheng)
tracking-fennec: ? → +

Comment 8

8 months ago
Joe, should we consider to fix add-on support in the next nightly cycle if no other priority? (looks similiar to 1396270, both are affecting the use of add-on)
Flags: needinfo?(wehuang)

Updated

7 months ago
See Also: → bug 1427255

Comment 9

7 months ago
Can we get a priority on this please? We think this is affecting add-ons and being mentioned in multiple other bugs.
(Assignee)

Updated

7 months ago
See Also: bug 1427255
Duplicate of this bug: 1427255
(Assignee)

Comment 11

7 months ago
Had a look at it - if this is actually as simple as it looks, I should have something within the next few days.
Assignee: nobody → jh+bugzilla
(Assignee)

Comment 12

7 months ago
As a quick fix I'd simply make those menu item objects parcelable, so we can stick them into the savedInstanceState and restore them from there, but when I can find some more time we actually have to move them out of BrowserApp in order to solve bug 1414084.
See Also: → bug 1414084

Comment 13

7 months ago
I want to add that in standard, not PWA mode, browserAction items disappear similarly to menu items. For example "Close button" from this extension https://addons.mozilla.org/firefox/addon/close-tab-button/
(Assignee)

Comment 14

7 months ago
For menu items the above approach seems to work fine. For URL bar page actions I need to see if I can get something similar working without too much hassle - if not, I'll punt it to bug 1414084.
(Assignee)

Comment 15

7 months ago
Respectively PageActions also have a Drawable that needs to be saved somehow and I'm not going down that rabbit hole right now, so definitively leaving PageActions to bug 1414084.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 19

7 months ago
mozreview-review
Comment on attachment 8945608 [details]
Bug 832990 - Part 0 - Improve comment wording.

https://reviewboard.mozilla.org/r/215742/#review222800
Attachment #8945608 - Flags: review+

Comment 20

7 months ago
mozreview-review
Comment on attachment 8945609 [details]
Bug 832990 - Part 1 - Make MenuItemInfo classes parcelable.

https://reviewboard.mozilla.org/r/215744/#review222806
Attachment #8945609 - Flags: review?(gkruglov) → review+

Comment 21

7 months ago
mozreview-review
Comment on attachment 8945610 [details]
Bug 832990 - Part 2 - Save and restore menu item caches via savedInstanceState.

https://reviewboard.mozilla.org/r/215746/#review222808

Looks good, but I'd like to hear an answer to Q2.

Q1: Can you add a comment in code saying that this is a stop-gag measure, and link to 1414084?

Q2: Also, it'd be good to leave brief comments explaining when these caches are actually used, and why you think they'll be populated by the time we need them. AFAICT, at two points - whenever we create the menu (so, after a user action to open, obviously cache will be in good state), and whenever messages like "Menu:AddBrowserAction" come in. The latter isn't obvious to me: from android docs, onRestoreInstanceState is called "between onStart() and onPostCreate(Bundle)." But we register listeners for these messages in "onCreate", leaving a bit of a gap. I'm not sure what the other side of this message bridge looks like - are you confident that small gap won't be problematic?
Attachment #8945610 - Flags: review?(gkruglov) → review+
(Assignee)

Comment 22

7 months ago
mozreview-review-reply
Comment on attachment 8945610 [details]
Bug 832990 - Part 2 - Save and restore menu item caches via savedInstanceState.

https://reviewboard.mozilla.org/r/215746/#review222808

Q1: Yes, of course.
Q2: That's why I'm restoring from within `onCreate()`, before those event listeners are even added.

Comment 23

7 months ago
(In reply to Jan Henning [:JanH] from comment #22)
> Q2: That's why I'm restoring from within `onCreate()`, before those event
> listeners are even added.

Ah, of course. Carry on then, I got things confused and thought you were going through 'onRestoreInstanceState'.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 27

7 months ago
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/8dee38a1aeec
Part 0 - Improve comment wording. r=Grisha
https://hg.mozilla.org/integration/autoland/rev/3f03169f58a9
Part 1 - Make MenuItemInfo classes parcelable. r=Grisha
https://hg.mozilla.org/integration/autoland/rev/fd684a8d5206
Part 2 - Save and restore menu item caches via savedInstanceState. r=Grisha
(Assignee)

Updated

7 months ago
Flags: needinfo?(jcheng)

Comment 28

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8dee38a1aeec
https://hg.mozilla.org/mozilla-central/rev/3f03169f58a9
https://hg.mozilla.org/mozilla-central/rev/fd684a8d5206
Status: NEW → RESOLVED
Last Resolved: 7 months ago
status-firefox60: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Is this something we should consider for Beta backport or can it ride the 60 train?
status-firefox57: affected → wontfix
status-firefox58: affected → wontfix
Flags: needinfo?(jh+bugzilla)

Comment 30

7 months ago
Please please backport, extensions are really painful to use on Android: I need to go back and fort in the add-ons panel to disable and re-enable NoScript almost every time I need to unblock a script.
(Assignee)

Comment 31

7 months ago
Comment on attachment 8945609 [details]
Bug 832990 - Part 1 - Make MenuItemInfo classes parcelable.

See below.
Attachment #8945609 - Flags: approval-mozilla-beta?
(Assignee)

Comment 32

7 months ago
Comment on attachment 8945610 [details]
Bug 832990 - Part 2 - Save and restore menu item caches via savedInstanceState.

Approval Request Comment
[Feature/Bug causing the regression]: Add-on menu item support on Android
[User impact if declined]: (Main) menu items added by add-ons disappear if Android  destroys the activity while Firefox is in background
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Not yet, but tested locally.
[Needs manual test from QE? If yes, steps to reproduce]: See comment 0, but instead of "Open the awesomebar screen" go into our settings or switch to some other app and then go back to Firefox.
[List of other uplifts needed for the feature/fix]: Part 1.
[Is the change risky?]: Not much.
[Why is the change risky/not risky?]: We're just passing the menu item data to Android for safekeeping when the activity is destroyed, so we can restore it later on if necessary. The data for a single menu item should take only around 100 bytes, so even for people with lots of add-ons we're very unlikely to exceed the maximum amount of data that can be stored that way (~500 kB).
[String changes made/needed]: none
Flags: needinfo?(jh+bugzilla)
Attachment #8945610 - Flags: approval-mozilla-beta?
Verified as fixed on Nightly 60.0a1, the add on is still displayed in Menu following the steps from comment 0 and comment 32.
Device: LG G4 (Android 5.1), Motorola Nexus 6 (Android 7.0).
status-firefox60: fixed → verified
Comment on attachment 8945609 [details]
Bug 832990 - Part 1 - Make MenuItemInfo classes parcelable.

Fixes a Fennec addon annoyance and verified on Nightly. Approved for 59b9.
Attachment #8945609 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8945610 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 36

6 months ago
Verified as fixed on the latest beta build, 59.0b9. This issue was verified on a Huawei P8 Lite - Android 8.0 and on a Pixel XL Android 8.0.0
status-firefox59: fixed → verified
(Assignee)

Updated

6 months ago
See Also: bug 1396270
Duplicate of this bug: 1396270

Comment 38

6 months ago
This is still happening in Firefox 60 Nightly. When a page loads, uBlock Origin and Decentraleyes entries sometimes aren't visible in the menu. I have to disable and enable extensions from Addons area for their entries to appear in menu.
(Assignee)

Comment 39

6 months ago
Are you using custom tabs or PWAs?
(In reply to smartfon.reddit from comment #38)
> This is still happening in Firefox 60 Nightly. When a page loads, uBlock
> Origin and Decentraleyes entries sometimes aren't visible in the menu. I
> have to disable and enable extensions from Addons area for their entries to
> appear in menu.

What device are you using? thanks
Flags: needinfo?(smartfon.reddit)

Comment 41

6 months ago
It's still happening for me too. 

LG G6, Android 7.0

I don't understand Jan's question about custom tabs, aren't they enabled by default? But I do have a PWA.
(Assignee)

Comment 42

6 months ago
(In reply to Paul [pwd] from comment #41)
> I don't understand Jan's question about custom tabs, aren't they enabled by
> default? But I do have a PWA.

Even if custom tabs are enabled in Firefox, you might not use any app that actually wants to open a custom tab in Firefox.

The reason I'm asking is because a variety of this bug can still happen if the Firefox process is started through a custom tab or a PWA, i.e. bug 1414084.

Comment 43

6 months ago
LG L65 with Lineage OS Android 7.1.2. Firefox 59.0b9
Happened when I opened link from other application (InoReader) using "Tab queue".

Comment 44

6 months ago
(In reply to Jan Henning [:JanH] from comment #42)
> (In reply to Paul [pwd] from comment #41)
> > I don't understand Jan's question about custom tabs, aren't they enabled by
> > default? But I do have a PWA.
> 
> Even if custom tabs are enabled in Firefox, you might not use any app that
> actually wants to open a custom tab in Firefox.
> 
> The reason I'm asking is because a variety of this bug can still happen if
> the Firefox process is started through a custom tab or a PWA, i.e. bug
> 1414084.

Well I'm guessing that's it then. I have Twitter and Flamingo amongst others using Firefox's custom tabs, as well as Hive Home set up as a PWA.
Unfortunately I don't have an LG G6 to test. We do have a LG G4 with Android 5.1 and could't reproduce. I've  installed uBlock, Stylish, Cookie AutoDelete and the menu items was still displayed after following the steps from comments. Tried also opening from tab queue and Custom tab.
Flags: needinfo?(smartfon.reddit)

Comment 46

6 months ago
I'm using LG G5. This happened with stock ROM as well as LineageOS.
I open links on Firefox by tapping a link in Flamingo for Twitter app. Flamingo's settings have "Chrome custom tabs" disabled. Firefox opens a regular tab with all available settings, and not a stripped down tab like in Chrome Custom Tabs.
Firefox is configured as the default phone Assistant from Android Settings. I don't use Progressive Web Apps. Custom Tabs entry is enabled in Firefox settings.

Comment 47

6 months ago
The problem happens with Custom Tabs option disabled in Firefox settings.
(Assignee)

Comment 48

6 months ago
Hmm,... please wait for bug 1414084 to be fixed, as that will rework that area of the code once more anyway.
If you can still reproduce any strange behaviour even after that bug lands, please open a new bug.
(Assignee)

Updated

5 months ago
Duplicate of this bug: 1444318
You need to log in before you can comment on or make changes to this bug.