Closed
Bug 1261375
Opened 8 years ago
Closed 8 years ago
'Pocket Saving' spins forever when moved to the menu panel and trying to save the New Tab Page
Categories
(Firefox :: Pocket, defect)
Tracking
()
VERIFIED
FIXED
Firefox 49
People
(Reporter: pauly, Assigned: mixedpuppy)
References
Details
Attachments
(2 files)
105.00 KB,
image/png
|
Details | |
58 bytes,
text/x-review-board-request
|
Gijs
:
review+
ritu
:
approval-mozilla-aurora+
ritu
:
approval-mozilla-beta+
|
Details |
[Affected versions]: - 46b6 [Affected platforms]: - all [Steps to reproduce]: 1. Start FX with a new profile 2. Enter 'Customize' mode and move the Pocket icon to the menu panel 3. Sign in to Pocket 4. Open the New Tab Page 5. Click on the Pocket button in the menu panel [Expected result]: - "Only links can be saved" error message should be displayed [Actual result]: - 'Saving' spins forever [Regression range]: - reproduced on FX 45, 48.0a1 (2016-03-31)
Updated•8 years ago
|
tracking-firefox46:
--- → ?
Keywords: regression
Assignee | ||
Comment 1•8 years ago
|
||
since this affects 45, I'm removing regression keyword.
Keywords: regression
Comment 2•8 years ago
|
||
Shane, Mike, double checking this doesn't block pocket as system addon shipping in 46. I'm assuming it doesn't and wontfixing for 46. Sounds like it may never have worked as expected. We should probably fix it though.
Flags: needinfo?(mixedpuppy)
Flags: needinfo?(mconnor)
Assignee | ||
Comment 3•8 years ago
|
||
[Tracking Requested - why for this release]: no, but lets track for 47
Tracked for 47, I hope we can uplift a fix soon, after 47.0b6 the uplift bar will be pretty high and only recent regressions, sec and stability issues will make the cut.
Hi Shane, please see comment 4. I hope we have a fix ready in the next week or so otherwise I will have to resolve this as a wontfix in the light of other more severe 47 tracked issues. :(
Flags: needinfo?(mixedpuppy)
Assignee | ||
Comment 6•8 years ago
|
||
fix is in progress. https://reviewboard.mozilla.org/r/49337/
Flags: needinfo?(mixedpuppy)
Assignee | ||
Comment 7•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/49337/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/49337/
Attachment #8746258 -
Flags: review?(gijskruitbosch+bugs)
Comment 8•8 years ago
|
||
Comment on attachment 8746258 [details] MozReview Request: Bug 1261375 fix load handling in pocket panel, r?gijs https://reviewboard.mozilla.org/r/49337/#review46275
Attachment #8746258 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment 9•8 years ago
|
||
At some point it might be good to have tests for some of this stuff...
Assignee: nobody → mixedpuppy
Status: NEW → ASSIGNED
Comment 10•8 years ago
|
||
(also, do we know why we thought the readyState stuff was necessary? I tested a bit and couldn't reproduce any issues with the patch, so this seems OK, but it's a little unsettling...)
(In reply to Shane Caraveo (:mixedpuppy) from comment #6) > fix is in progress. https://reviewboard.mozilla.org/r/49337/ Thanks! Really appreciate the prompt follow up.
Assignee | ||
Comment 12•8 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #10) > (also, do we know why we thought the readyState stuff was necessary? I > tested a bit and couldn't reproduce any issues with the patch, so this seems > OK, but it's a little unsettling...) readyState is a pattern I use often with iframes in panels to make sure the iframe load doesn't beat onshown. In any case, the new patch I just pushed keeps this and just checks that the uri is not about:blank. It always seems to be about:blank, but this would catch an edge case [wouldn't surprise me if there is one].
Assignee | ||
Comment 13•8 years ago
|
||
Comment on attachment 8746258 [details] MozReview Request: Bug 1261375 fix load handling in pocket panel, r?gijs Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49337/diff/1-2/
Assignee | ||
Comment 14•8 years ago
|
||
ping for updated patch review
Flags: needinfo?(gijskruitbosch+bugs)
Comment 15•8 years ago
|
||
This also looks good to me. Sorry, I did not get flagged for review again because mozreview assumed my review would carry (there isn't currently a way for you to fix this as the patch submitter. See e.g. bug 1247772, bug 1247069, ...)
Flags: needinfo?(gijskruitbosch+bugs)
Assignee | ||
Comment 16•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/ca1a70889b2bc2009bb18b831598fec26a485919 Bug 1261375 fix load handling in pocket panel, r=gijs
Comment 17•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ca1a70889b2b
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Assignee | ||
Comment 18•8 years ago
|
||
Comment on attachment 8746258 [details] MozReview Request: Bug 1261375 fix load handling in pocket panel, r?gijs Approval Request Comment [Feature/regressing bug #]:pocket [User impact if declined]:In some instances pocket will fail (timing related) [Describe test coverage new/current, TreeHerder]:on central [Risks and why]: low, this just adds a check that the loaded document is not about:blank [String/UUID change made/needed]: none
Attachment #8746258 -
Flags: approval-mozilla-beta?
Attachment #8746258 -
Flags: approval-mozilla-aurora?
Reporter | ||
Comment 19•8 years ago
|
||
Verified fixed FX 49.0a1 (2016-05-08) Win 7, Ubuntu 14.04, OS X 10.10.5
Status: RESOLVED → VERIFIED
Comment on attachment 8746258 [details] MozReview Request: Bug 1261375 fix load handling in pocket panel, r?gijs Recent regression in Pocket. Fix was verified on Nightly, Aurora48+, Beta47+
Attachment #8746258 -
Flags: approval-mozilla-beta?
Attachment #8746258 -
Flags: approval-mozilla-beta+
Attachment #8746258 -
Flags: approval-mozilla-aurora?
Attachment #8746258 -
Flags: approval-mozilla-aurora+
Comment 21•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/2d2c08b645df
Comment 22•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/3ea4c28099c5
Reporter | ||
Comment 24•8 years ago
|
||
Verified fixed FX 47b8, 48.0a2 (2016-05-26) Win 7.
You need to log in
before you can comment on or make changes to this bug.
Description
•