Closed Bug 1292081 Opened 3 years ago Closed 3 years ago

Refreshing the page from inside the toolbox triggers two post confirmation dialog boxes

Categories

(DevTools :: General, defect)

49 Branch
defect
Not set

Tracking

(firefox48 unaffected, firefox49+ fixed, firefox50+ fixed, firefox51 verified)

VERIFIED FIXED
Firefox 51
Tracking Status
firefox48 --- unaffected
firefox49 + fixed
firefox50 + fixed
firefox51 --- verified

People

(Reporter: darktrojan, Assigned: ochameau)

References

Details

(Keywords: regression)

Attachments

(2 files)

STR:

Open a page with an HTTP POST request.
Open the developer toolbox.
Press F5.

The page tries to reload twice, opening two confirmation dialogs.

The double reload probably happens with GET requests as well, but it's far more obvious with POST.
I've just tried this in Firefox 49, and it doesn't happen there.
Keywords: regression
Attached file form-post.html
Here's STR:

Open this page
Submit the form
Reload the page (one prompt)
Open devtools
Reload the page again (two prompts)
I see this behavior on both 49 and 50.  Can you confirm you aren't seeing the behavior in 49?
Flags: needinfo?(geoff)
Huh. Okay, I can see it in 49 as well. I thought my update to 49b1 had already happened, but apparently it had not. Sorry about that.
Flags: needinfo?(geoff)
It seems like this is actually a regression, but you have go back one more cycle.  48 is unaffected and I can reproduce in 49 and later.  e10s does not seem to matter here, I can see the issue with and without it.
Attached patch patch v1Splinter Review
I'm wondering if there is any value to register these key shortcuts when host isn't window and the target is local tab.
But may be we want to ensure we always reload throught our codepath.
Attachment #8779727 - Flags: review?(jryans)
Thanks for the report and the regression range!
Comment on attachment 8779727 [details] [diff] [review]
patch v1

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

Great, thanks for the fix!  Let's try to get this uplifted.
Attachment #8779727 - Flags: review?(jryans) → review+
[Tracking Requested - why for this release]: DevTools bug triggers the page to be reloaded twice instead of once as intended.
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/39fdb9c2017e
Prevents reloading web pages twice when devtools are opened. r=jryans
https://hg.mozilla.org/integration/fx-team/rev/39fdb9c2017e96940075926234809cef0f8b5f1b
Bug 1292081 - Prevents reloading web pages twice when devtools are opened. r=jryans
Assignee: nobody → poirot.alex
https://hg.mozilla.org/mozilla-central/rev/39fdb9c2017e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Hello Alexandre, could you please nominate patch for uplift to Aurora and Beta as 50, 49 are marked as affected?
Flags: needinfo?(poirot.alex)
Comment on attachment 8779727 [details] [diff] [review]
patch v1

Approval Request Comment
[Feature/regressing bug #]: bug 1268450
[User impact if declined]: when devtools are opened, pressing reload key shortcuts is going to reload the page twice and on pages submitted using POST request, the reload prompt will appear twice.
[Describe test coverage new/current, TreeHerder]: on master for a few days, tried to introduce a specific unit test but it is quite challenging to test automatically. Otherwise these key shortcuts are covered by unit test, we just don't check the duplicated loads possiblity.
[Risks and why]: Quite simple patch, restricted to devtools codebase.
[String/UUID change made/needed]: no
Flags: needinfo?(poirot.alex)
Attachment #8779727 - Flags: approval-mozilla-beta?
Attachment #8779727 - Flags: approval-mozilla-aurora?
Hello Geoff, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(geoff)
Comment on attachment 8779727 [details] [diff] [review]
patch v1

This is a pretty core scenario, fix has stabilized on Nightly for 2 days, Aurora50+
Attachment #8779727 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8779727 [details] [diff] [review]
patch v1

Fix for regression in 49, the risk seems low, let's uplift for beta 6.
Attachment #8779727 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Ritu Kothari (:ritu) from comment #17)
> Hello Geoff, could you please verify this issue is fixed as expected on a
> latest Nightly build? Thanks!

Looks good to me.
Flags: needinfo?(geoff)
Status: RESOLVED → VERIFIED
Version: unspecified → 49 Branch
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.