Closed
Bug 1164253
Opened 9 years ago
Closed 9 years ago
Save request is sent twice for every button press
Categories
(Firefox :: Pocket, defect, P1)
Firefox
Pocket
Tracking
()
VERIFIED
FIXED
Firefox 41
People
(Reporter: nate+bugzilla, Assigned: Dolske)
References
Details
Attachments
(1 file)
746 bytes,
patch
|
jaws
:
review+
Dolske
:
approval-mozilla-aurora+
Dolske
:
approval-mozilla-beta+
Dolske
:
approval-mozilla-release+
|
Details | Diff | Splinter Review |
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•9 years ago
|
||
*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)
Reporter | ||
Comment 2•9 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 3•9 years ago
|
||
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•9 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+
Assignee | ||
Comment 6•9 years ago
|
||
(aurora currently closed) https://hg.mozilla.org/releases/mozilla-beta/rev/c2f420841c18 https://hg.mozilla.org/releases/mozilla-release/rev/89ef57a1733a
https://hg.mozilla.org/mozilla-central/rev/494fc85d8aef
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Comment 9•9 years ago
|
||
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
Comment 10•9 years ago
|
||
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.
Description
•