Closed
Bug 832990
Opened 12 years ago
Closed 7 years ago
Menu items added by add-ons disappear if "Don't keep activities" is checked
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(fennec+, firefox57 wontfix, firefox58 wontfix, firefox59 verified, firefox60 verified)
RESOLVED
FIXED
Firefox 60
People
(Reporter: kats, Assigned: JanH)
References
Details
(Whiteboard: [add-ons])
Attachments
(3 files)
(deleted),
text/x-review-board-request
|
Grisha
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
Grisha
:
review+
RyanVM
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-review-board-request
|
Grisha
:
review+
RyanVM
:
approval-mozilla-beta+
|
Details |
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.
Reporter | ||
Updated•12 years ago
|
Reporter | ||
Comment 1•9 years ago
|
||
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: --- → ?
Comment 2•9 years ago
|
||
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•9 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]
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•7 years 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•7 years ago
|
Assignee | ||
Updated•7 years ago
|
tracking-fennec: + → ?
status-firefox57:
--- → affected
status-firefox58:
--- → affected
status-firefox59:
--- → affected
Version: unspecified → Trunk
Comment 7•7 years ago
|
||
Hi Joe, Wesly
Please help prioritize this.
Flags: needinfo?(wehuang)
Flags: needinfo?(jcheng)
Updated•7 years ago
|
tracking-fennec: ? → +
Comment 8•7 years 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 years ago
|
Comment 9•7 years 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 years ago
|
Assignee | ||
Comment 11•7 years 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 years 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.
Comment 13•7 years 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 years 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 years 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 years 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 years 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 years 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 years 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 years 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 years 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 years ago
|
Flags: needinfo?(jcheng)
Comment 28•7 years 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
Closed: 7 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Comment 29•7 years ago
|
||
Is this something we should consider for Beta backport or can it ride the 60 train?
Flags: needinfo?(jh+bugzilla)
Comment 30•7 years 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 years 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 years 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?
Comment 33•7 years ago
|
||
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).
Comment 34•7 years ago
|
||
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+
Updated•7 years ago
|
Attachment #8945610 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 35•7 years ago
|
||
bugherder uplift |
Comment 36•7 years 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
Assignee | ||
Updated•7 years ago
|
Comment 38•7 years 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•7 years ago
|
||
Are you using custom tabs or PWAs?
Comment 40•7 years ago
|
||
(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•7 years 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•7 years 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•7 years 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•7 years 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.
Comment 45•7 years ago
|
||
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•7 years 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•7 years ago
|
||
The problem happens with Custom Tabs option disabled in Firefox settings.
Assignee | ||
Comment 48•7 years 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.
Comment 50•6 years ago
|
||
Opened #1505617 as I still have this behavior on Android 8.1 Nexus 6P with Firefox 62.0.3.
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•