Save request is sent twice for every button press

VERIFIED FIXED in Firefox 38.0.5

Status

()

defect
P1
normal
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: nate, Assigned: Dolske)

Tracking

unspecified
Firefox 41
Points:
---

Firefox Tracking Flags

(firefox38.0.5 verified, firefox39 fixed, firefox40 fixed, firefox41 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
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 :)
(Assignee)

Comment 1

4 years ago
Posted 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)
(Assignee)

Updated

4 years ago
Blocks: 1163319
(Reporter)

Comment 2

4 years ago
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+
(Assignee)

Comment 5

4 years ago
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
Last Resolved: 4 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.