Last Comment Bug 1171569 - browser.pocket.enabled doesn't fully disable the Pocket integration ("View Pocket List" entry remaining in bookmarks menu)
: browser.pocket.enabled doesn't fully disable the Pocket integration ("View Po...
Status: RESOLVED WORKSFORME
:
Product: Firefox
Classification: Client Software
Component: Pocket (show other bugs)
: 38 Branch
: All All
-- normal with 10 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
: Shane Caraveo (:mixedpuppy)
Mentors:
: 1172253 1176888 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-06-04 10:47 PDT by Mark Schmidt :marksc
Modified: 2015-08-30 08:25 PDT (History)
18 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
affected
affected
affected


Attachments

Description User image Mark Schmidt :marksc 2015-06-04 10:47:29 PDT
Setting browser.pocket.enabled to 'false' properly removes the Pocket button. However, it does not remove the "view pocket list" item which can be seen when left-clicking on the bookmarks menu button.

Some users find the current behavior alarming. They see a third-party in their browser which they didn't ask to be there and which they can't completely remove, and that worries them.
Comment 1 User image Justin Dolske [:Dolske] 2015-06-08 12:48:41 PDT
The normal/supported way to remove Pocket works fine: Right click the button, select "Remove from toolbar".

https://support.mozilla.org/en-US/kb/disable-pocket-firefox

The .enabled pref doesn't really do anything more, and there really no reason to use it. But we should still probably fix this.
Comment 2 User image Justin Dolske [:Dolske] 2015-06-08 12:49:01 PDT
*** Bug 1172253 has been marked as a duplicate of this bug. ***
Comment 3 User image rsx11m 2015-06-09 04:18:06 PDT
Note that browser.pocket.enabled set to "false" doesn't really disable the feature (the bookmark entry remains fully functional, afaict) but just hides the UI elements. The backend needs to be deactivated as well to satisfy the user's expectation on what this preference actually does.
Comment 4 User image Justin Dolske [:Dolske] 2015-06-09 12:38:43 PDT
(In reply to rsx11m from comment #3)
> The backend needs to be deactivated as well to satisfy the
> user's expectation on what this preference actually does.

There is no "backend" to disable. Pocket-related code only runs when the Pocket button is clicked -- so no button, no code.

The bug here is simply that some of the integration points (menu entries) only check to see if the Pocket button has been removed or not, and setting .enabled to false essentially just hides the button instead of removing it.

We should fix this bug by removing the browser.pocket.enabled pref, and make sure that anyone who has the pref set to false automagically has the button removed as if they had done the "Remove From Toolbar" action.
Comment 5 User image msth67 2015-06-10 12:50:51 PDT Comment hidden (off-topic)
Comment 6 User image Ricardo 2015-06-12 11:49:31 PDT
Will this be done for 39 release?

http://mxr.mozilla.org/mozilla-release/source/browser/base/content/browser-places.js#1434
> let hidden = !CustomizableUI.getPlacementOfWidget("pocket-button") || !Services.prefs.getBoolPref("browser.pocket.enabled");
Comment 7 User image Loic 2015-06-24 10:06:04 PDT
*** Bug 1176888 has been marked as a duplicate of this bug. ***
Comment 8 User image rsx11m 2015-06-27 12:01:43 PDT
(In reply to Justin Dolske [:Dolske] from comment #4)
> We should fix this bug by removing the browser.pocket.enabled pref, and make
> sure that anyone who has the pref set to false automagically has the button
> removed as if they had done the "Remove From Toolbar" action.

How would this work from the central-deployment perspective? If you want certain preferences disabled by default (e.g., an employer doesn't want their users to use Hello/Loop or Pocket and disables it in a central installation), you'd just set the respective pref (loop.enable or browser.pocket.enabled) and lock it, then you are done. In contrast, browser.uiCustomization.state can't be fixed in this way as it needs to be customizable in a user's profile.

Personally, I'd prefer the solution suggested in comment #6, which seems easy enough. It also would be rather confusing if some features have an easy ".enable"/".disable" pref whereas others don't.
Comment 9 User image rsx11m 2015-07-21 16:27:28 PDT
Was this fixed as part of mozilla-central changeset 8d96b303d33a from bug 1164942 - do not load pocket's jsm or other scripts until used with extra getter to satisfy tests?

I can no longer reproduce in a current 42.0a1 build. Setting browser.pocket.enabled to false doesn't have any immediate effect, but upon restart hides /both/ the Toolbar and Bookmarks menu buttons.

Apparently "pocket-button" is no longer loaded in CustomizableUI at startup with the pref set to false and hence the test on it being present fails in BookmarkingUI.updatePocketItemVisibility() later.

FWIW, I've tested the proposed fix in comment #6 and it works for the purpose of hiding the Bookmarks menuitem unconditionally, but that solution may be obsolete now with the fix from the other bug.
Comment 10 User image rsx11m 2015-07-21 16:36:12 PDT
Ok, it's no longer reproducible in the current 39.0 release either, so that's probably a WFM now.
Comment 11 User image wsha.code 2015-07-24 06:22:10 PDT
With the browser.pocket.enabled set to 'false', a Pocket button still appears in Reader View on the left sidebar. This button should probably be hidden as well.
Comment 12 User image Phoenix 2015-08-15 13:15:13 PDT
Resolved per whiteboard and Comment 10
Comment 13 User image hartnegg 2015-08-30 08:25:55 PDT
There is another case in which setting browser.pocket.enabled to false does not remove the pocket entry from the bookmark menu: see bug 1197871. Does not involve previous use of beta versions.

What does help is: set browser.pocket.enabled to true, restart Firefox, manually add the icon to the toolbar, manually remove it from the toolbar.

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