Closed Bug 1164253 Opened 5 years ago Closed 5 years ago

Save request is sent twice for every button press

Categories

(Firefox :: Pocket, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 41
Tracking Status
firefox38.0.5 --- verified
firefox39 --- fixed
firefox40 --- fixed
firefox41 --- fixed

People

(Reporter: nate+bugzilla, Assigned: Dolske)

References

Details

Attachments

(1 file)

For each save with the toolbar button, two save requests are being sent to the server. 

The second save is happening when the panel closes, so I'm assuming this is a regression that we didn't catch caused by the last minute changes to event handling over the weekend. 

Steps to reproduce:
1. Open the Browser Toolbox
2. Switch to the Network tab
3. Click the Pocket button in the toolbar
4. Note in the network log that a save request shows
5. Click in the window to dismiss the Pocket panel
6. Note in the network log a second save request goes out when the panel closes

Marking as a P1 as this is not going to be very nice to our servers :)
Attached patch Patch v.1Splinter Review
*cough* Mea culpa.

Towards the end of bug 1163319 I renamed a "onPanelLoaded" function to "onFrameLoaded" to better reflect what was happening, but missed a rename. And so we're not actually removing the event listener. Oops.

[The iframe's @src is being set to about:blank when the panel closes, and the load event from that is what's causing the second call.]
Assignee: nobody → dolske
Attachment #8604996 - Flags: review?(jaws)
Blocks: 1163319
Applied the patch, built, tested, confirmed fixed. 

Come on Dolske, it's like you were writing last-minute code at 10pm on a Saturday night and didn't have anyone to back you up to help test a bug you'd only notice if you had the network console open ;)

Thanks for the quick fix.
Comment on attachment 8604996 [details] [diff] [review]
Patch v.1

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

D'oh, guess I missed it in my review as well.
Attachment #8604996 - Flags: review?(jaws) → review+
Comment on attachment 8604996 [details] [diff] [review]
Patch v.1

[Triage Comment]

a+ for aurora/beta/release: required for Pocket launch in 38.0.5.
Attachment #8604996 - Flags: approval-mozilla-release+
Attachment #8604996 - Flags: approval-mozilla-beta+
Attachment #8604996 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/494fc85d8aef
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Reproduced the bug with 38.0.5b1 (20150511143336), confirmed fixed on 38.0.5b3 (20150518141916).
Status: RESOLVED → VERIFIED
Flags: qe-verify+
QA Contact: andrei.vaida
Already verified for Firefox 38.0.5. I don't think verification needs to be repeated for the other versions.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.