Closed Bug 1163917 Opened 9 years ago Closed 9 years ago

The built-in Pocket is NOT completely disabled if Pocket is already installed and used as an add-on

Categories

(Firefox :: Pocket, defect, P1)

defect
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 41
Iteration:
41.1 - May 25
Tracking Status
firefox38.0.5 --- verified
firefox39 --- fixed
firefox40 --- fixed
firefox41 --- fixed

People

(Reporter: avaida, Assigned: jaws)

References

Details

Attachments

(1 file)

Reproducible on:
* Firefox 38.0.5b1 (build1: 20150510205200)
* Firefox 38.0.5b1 (build2: 20150511143336)

Affected platforms:
Windows 8.1 (x86), Windows 7 (x64), Mac OS X 10.9.5, Ubuntu 14.04 (x64)

Preconditions:
* the user has a profile with Pocket installed as an add-on from AMO

Steps to reproduce:
1. Launch Firefox with built-in Pocket using that profile.
2. Open a page that's currently NOT in your Pocket list. Right-click somewhere in that page and click "Save to Pocket".
3. [Linux only]: From the Bookmarks Menu, click "View Pocket List".

Expected result:
* The Pocket add-on is used instead of the built-in one.
* The built-in Pocket button is NOT visible in the toolbar.
* The "View Pocket List" menuitem from Bookmarks Menu is NOT visible.
* The "Save to Pocket" context menu options is NOT visible for eligible pages.

Actual result:
The Pocket add-on is indeed used instead of the built-in one, but the following built-in functionalities remain visible:
* on Linux: the "View Pocket List" menuitem from Bookmarks Menu, the "Save to Pocket" context menu option, the "Save Link to Pocket" context menu option
* on Mac: the "Save Link to Pocket" context menu option
* on Windows: the "Save Link to Pocket" context menu option

Additional notes:
* The "Save to Pocket" and "Save Link to Pocket" context menu options actually work, although the add-on is installed and used instead of the built-in one.
* On Linux, clicking the "View Pocket List" menuitem from the Bookmarks Menu points the user to Pocket's sign-up/sign-in page (although the user is already signed in with the add-on).
Priority: -- → P1
(Pocket helped review this for the beta1, not as scary as it sounded but we should definitely fix.)
Assignee: nobody → jaws
Status: NEW → ASSIGNED
I can't reproduce this.

Note that the initial bug report mentions a "Save Link to Pocket". This was not part of the Firefox integration until today (just landed on fx-team and hasn't made it to a Firefox Nightly build yet).

"Save to Pocket" and "Save Link to Pocket" context menuitems are also part of the add-on, so this may be why it was thought that they weren't disabled.

Can you retest this with the builds from https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=d5a378327e88 ? 
Linux: http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-team-linux/1431626703/
Windows: http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-team-win32/1431626703/
OSX: http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-team-macosx64/1431626703/
Flags: needinfo?(andrei.vaida)
Maybe related.  Had Pocket 1.0 add-on enabled and after updating to 38.0.5 I tried to save pages by clicking the Pocket toolbar icon.  Until I disabled the Pocket 1.0 add-on, I saw these results:

https://www.dropbox.com/s/p69v8pio3t84ofi/chadw.png?dl=0
http://cl.ly/image/3G1m3V2P1N2G

After disabling, I saw expected results.
With Firefox 38.0.5 Beta 2 (BuildID=20150514163436) on Mac OS X 10.8.5 the following options are visible:
- "View Pocket List" menuitem from Bookmarks Menu - leads to Pocket Sign Up page
- "Save to Pocket" context menu option - saves page to Add-on
- "Save Link to Pocket" context menu option - saves link to Add-on
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #2)
> Can you retest this with the builds from
> https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=d5a378327e88 ? 

Encountered the following results when launching the builds pointed in comment 2 with a profile that has Pocket add-on 3.0.6 installed and logged in, under Windows 7 64-bit, Mac OS X 10.9.5 and Ubuntu 12.04 32-bit:
* Pocket add-on is used instead of the built-in one
* 'Save to Pocket' option displayed in context menu 
* 'View Pocket List' menu item present in Bookmarks Menu; Clicking it points the user to Pocket's sign-up/sign-in page (although the user is already signed in with the add-on)

Please let me know if I can help more!
Flags: needinfo?(andrei.vaida)
Attached patch PatchSplinter Review
This is because the conditionalDestroyPromise only "destroys" the widget but doesn't remove it from its placement. The various touch points in the code that check to see if the built-in version of Pocket is enabled look to see if it is placed outside of the palette. Destroy doesn't remove the item from its placement, so those checks erroneously assume that it is enabled.

This patch changes the behavior of the conditionalDestroyPromise to remove the item from its placement too. IMO, having a widget that is removed from the DOM but still has other APIs reporting that it exists is misleading. I didn't change the name of the promise though, because I wanted to keep the patch smaller and also didn't know of a better name (conditionalRemovalPromise perhaps?).
Attachment #8607097 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8607097 [details] [diff] [review]
Patch

Review of attachment 8607097 [details] [diff] [review]:
-----------------------------------------------------------------

D'oh.
Attachment #8607097 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8607097 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: pocket
[User impact if declined]: various parts of the built-in pocket integration will remain enabled when the user has the pocket add-on installed
[Describe test coverage new/current, TreeHerder]: manual testing
[Risks and why]: low risk, only related to pocket
[String/UUID change made/needed]: none
Attachment #8607097 - Flags: approval-mozilla-release?
Attachment #8607097 - Flags: approval-mozilla-beta?
Attachment #8607097 - Flags: approval-mozilla-aurora?
Iteration: --- → 41.1 - May 25
Points: --- → 2
Flags: qe-verify+
Flags: firefox-backlog+
Comment on attachment 8607097 [details] [diff] [review]
Patch

Taking it now to make sure it is in 38.0.5
Attachment #8607097 - Flags: approval-mozilla-release?
Attachment #8607097 - Flags: approval-mozilla-release+
Attachment #8607097 - Flags: approval-mozilla-beta?
Attachment #8607097 - Flags: approval-mozilla-beta+
Attachment #8607097 - Flags: approval-mozilla-aurora?
Attachment #8607097 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/6d3a597b0935
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
With 38.0.5 beta 3 (Build ID: 20150518141916), across platforms, I see the same results as mentioned in comment 5. Could you please shed some light on this? Thanks in advance!
Flags: needinfo?(jaws)
(In reply to Alexandra Lucinet, QA Mentor [:adalucinet] from comment #5)
> Encountered the following results when launching the builds pointed in
> comment 2 with a profile that has Pocket add-on 3.0.6 installed and logged
> in, under Windows 7 64-bit, Mac OS X 10.9.5 and Ubuntu 12.04 32-bit:
> * Pocket add-on is used instead of the built-in one

This is intended behavior.

> * 'Save to Pocket' option displayed in context menu 

This is correct. The add-on uses the text "Save to Pocket" whereas the built-in implementation uses "Save Page to Pocket".

> * 'View Pocket List' menu item present in Bookmarks Menu; Clicking it points
> the user to Pocket's sign-up/sign-in page (although the user is already
> signed in with the add-on)

You may need to go to about:config and reset the `browser.toolbarbuttons.introduced.pocket-button` pref (right-click and choose Reset). Then restart the browser, and it should fix this. This menuitem should no longer appear when the add-on is installed and active.
Flags: needinfo?(jaws)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #16)
> > * 'View Pocket List' menu item present in Bookmarks Menu; Clicking it points
> > the user to Pocket's sign-up/sign-in page (although the user is already
> > signed in with the add-on)
> 
> You may need to go to about:config and reset the
> `browser.toolbarbuttons.introduced.pocket-button` pref (right-click and
> choose Reset). Then restart the browser, and it should fix this. This
> menuitem should no longer appear when the add-on is installed and active.

Indeed, by resetting 'browser.toolbarbuttons.introduced.pocket-button' pref, 'View Pocket List' is no longer available in Bookmarks Menu. But, isn't it supposed to do that on its own? Do we have a plan to implement this later on? End users are most likely not going to do this. They shouldn't need to.
Flags: needinfo?(jaws)
Beta users would need to manually flip that pref, but users on Release Firefox won't need to flip the pref. This is because beta users have already had the button "installed" but Release users haven't gotten it yet.
Flags: needinfo?(jaws)
Closing this based on Comment 17 and Comment 18. Follow-up issues, if any, will be filed separately.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.