Closed Bug 1498914 Opened 3 years ago Closed 3 years ago

Removal of Pocket icon from address bar does not persist across a restart


(Firefox :: Toolbars and Customization, defect, P1)




Firefox 64
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 --- unaffected
firefox64 + verified


(Reporter: mstanke, Assigned: thecount)



(Keywords: nightly-community, regression)


(1 file)

+++ This bug was initially created as a clone of Bug #1483033 +++

The same what happened with Screenshots page action is now happening with Pocket page action. After rester, it shows in the address bar again, despite I have hidden it the previous run.
Duplicate of this bug: 1498895
[Tracking Requested - why for this release]:
Functionality regression that we shouldn't ship (ideally, even to beta).

Scott, can you investigate this? Drew (:adw) should be able to help if you're stuck, and bug 1483033 might offer clues about why this is broken.
Blocks: 1488813
Flags: needinfo?(sdowne)
I can look into this.
Flags: needinfo?(sdowne)
One thing that sticks out to me is this is no longer an extension. It was moved into a component, which is what broke this.

I'm not sure if the fixes in bug 1483033 would still apply, given those fixes were for an extension? I'll take a look anyway, but any info Drew has around doing something similar without it being an extension would be appreciated. :)
Flags: needinfo?(adw)
Assignee: nobody → sdowne
From more investigation, it looks like it has something to do with when to call shutdown and when to not, based on the old extension code's reason value? If shutdown is called, then next startup re adds the icon. Looks at the logic of the patch for screenshots, this would make sense.
It looks like you can remove the pageAction.remove() call here:

remove() is really only for actions that live in extensions or code that can otherwise be disabled (like behind a pref).  remove() tells the page actions machinery to forget about the action, including whether the action is in the urlbar or not.  If Pocket isn't an extension anymore -- or at least, if it can no longer be disabled -- then you probably don't ever want to call that.  In fact that whole shutdown() method doesn't look necessary anymore.  I wouldn't think you would ever need to remove #pocket-button-box either.
Flags: needinfo?(adw)
I noticed that when the browser is not running, prefs.js does not have "pocket" in the list of "ids" for browser.pageActions.persistedActions like it did before. Manually adding it and starting the browser causes the page action to behave correctly and abide by "idsInUrlbar" until the next restart.
Yes, that's due to the remove() call, which makes Firefox forget about the Pocket page action.
Great, this all makes sense to me now, I should have a fix in a bit.
@adw Cool if I request review from you for this?
Flags: needinfo?(adw)
Thanks, r+'ed
Flags: needinfo?(adw)
Keywords: checkin-needed
Pushed by
Removal of Pocket icon from address bar persist across a restart r=adw
Keywords: checkin-needed
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Hi, I checked this issue in Firefox Nightly 64.0a1 (2018-10-19) and the issue no longer occurs, I will mark this accordingly.

This does not work on FF Nightly 74.0a1. Should I open a new issue?

Yeah, a new issue would be ideal.

Also I just tried this on nightly, and the Pocket icon removal does persist across a restart on my end.

Do you have any steps I need to do to reproduce the issue?

Also are you using firefox sync? It's just a guess, but maybe there is a bug with sync, and if you don't have sync setup that would quickly rule that out as a potential cause.

Okay, I have created a new issue #1611516.

I tried to list the steps in that issue.

And yes, I am using Firefox sync.

I tried doing this with Pocket on my end and it stays gone. I'll see if I can reproduce the bug playing around with sync settings.

You need to log in before you can comment on or make changes to this bug.