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

Status: VERIFIED FIXED

(DevTools :: General, defect)

Description

[Geoff Lankow (:darktrojan)]

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.

Comment 1

[Geoff Lankow (:darktrojan)]

I've just tried this in Firefox 49, and it doesn't happen there.

Comment 2

[Brian Grinstead [:bgrins]]

Attachment: 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)

Comment 3

[Brian Grinstead [:bgrins]]

I see this behavior on both 49 and 50.  Can you confirm you aren't seeing the behavior in 49?

Comment 4

[Geoff Lankow (:darktrojan)]

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.

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 6

[Alice0775 White]

I can reproduce the problem if I used keyboard shortcut(F5) to reload page.

Regression window:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=fef64183cb939b13fbfdd5b1b44bd3c63d64d88f&tochange=df627da479f868c9704d7ac0b1c6aa76fb298150

Regressed by: Bug 1268450

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: patch v1

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.

Comment 8

[Alexandre Poirot [:ochameau]]

Thanks for the report and the regression range!

Comment 9

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=70dfc37012d1

Comment 10

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 11

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

[Tracking Requested - why for this release]: DevTools bug triggers the page to be reloaded twice instead of once as intended.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/39fdb9c2017e
Prevents reloading web pages twice when devtools are opened. r=jryans

Comment 13

[Alexandre Poirot [:ochameau]]

https://hg.mozilla.org/integration/fx-team/rev/39fdb9c2017e96940075926234809cef0f8b5f1b
Bug 1292081 - Prevents reloading web pages twice when devtools are opened. r=jryans

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/39fdb9c2017e

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hello Alexandre, could you please nominate patch for uplift to Aurora and Beta as 50, 49 are marked as affected?

Comment 16

[Alexandre Poirot [:ochameau]]

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

Comment 17

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hello Geoff, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8779727 [details] [diff] [review]
patch v1

This is a pretty core scenario, fix has stabilized on Nightly for 2 days, Aurora50+

Comment 19

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8779727 [details] [diff] [review]
patch v1

Fix for regression in 49, the risk seems low, let's uplift for beta 6.

Comment 20

[Geoff Lankow (:darktrojan)]

(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.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c6110450ff8f

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/592c0b51f175