796661 - [Browser] Confirm reloading POST request

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Browser, defect, P1)

Description

[GH to BZ]

[GitHub issue by timdream on 2012-09-17T17:09:41Z, https://github.com/mozilla-b2g/gaia/issues/4817]
As a user I want to be asked to confirm whether or not I want to reload a page if it will re-send POST data, so I don't accidentally submit my order for a new Ferrari twice


Was: [Browser] Unable to reload a page coming from POST request, no warning shown

STR:

1. Open up Browser, go to http://pda.5284.com.tw/MQS/businfo2.jsp?routename=555
2. Click on the "立即更新" (refresh now) button, which submits a POST request form but loads the same page.
3. Attempt to reload by press the reload button on the browser chrome.

Expected:

1. A prompt should show up with warning text, saying something like (e.g., on Desktop Firefox)

    To display this page, Firefox must send information that will repeat any action (such as a search or order confirmation) that was performed earlier. [Reload] [Cancel]

2. Upon pressing [Reload], the page reloads. Press [Cancel] and the page will not reload

Actual:

1. The page is not reload nor any warning shows.

Note:

@benfrancis We'll need platform `mozbrowserXXX` event for this. And we need to implement the UI 3 times. Yeah!

Comment 1

[GH to BZ]

[GitHub comment by timdream on 2012-09-17T17:10:27Z]
Fortunately this doesn't result crash like the HTTP auth dialog did ... but this is definitely a blocker.

Comment 2

[GH to BZ]

[GitHub comment by benfrancis on 2012-09-24T10:46:51Z]
Sigh. Good catch Tim.

Comment 3

[GH to BZ]

[GitHub comment by benfrancis on 2012-09-24T11:03:47Z]
I've filed https://bugzilla.mozilla.org/show_bug.cgi?id=793644 for the platform part.

Also marking needUXInput but we'll probably just need some copy and button labels for a simple dialog, should this be marked as a blocker.

Comment 4

[GH to BZ]

[GitHub comment by autonome on 2012-09-24T15:35:47Z]
@timdream does focusing the URL bar and hitting <enter> reload that page?

Comment 5

[GH to BZ]

[GitHub comment by benfrancis on 2012-09-24T15:45:58Z]
Yes it does, it will just send a GET rather than a POST request.

Comment 6

[GH to BZ]

[GitHub comment by benfrancis on 2012-09-24T17:54:44Z]
@autonome FYI the platform part of this just got marked blocking-basecamp+ https://bugzilla.mozilla.org/show_bug.cgi?id=793644

Comment 7

[GH to BZ]

[GitHub comment by autonome on 2012-09-26T16:16:34Z]
Is there any Gaia work for this once the platform issue lands?

Comment 8

[GH to BZ]

[GitHub comment by benfrancis on 2012-09-26T16:17:16Z]
Yes, about the same as HTTP Auth I would imagine. Estimating as LOE:S

Comment 9

[Alex Keybl [:akeybl]]

[mass adding reproducible keyword for any open Gaia bug with the word "STR:" in comments]

Comment 10

[Alex Keybl [:akeybl]]

We're marking this bug with the C1 milestone since it follows the criteria of "unfinished feature work" (see https://etherpad.mozilla.org/b2g-convergence-schedule).

If this work is not finished by Nov19, this bug will need an exception and will be called out at the upcoming Exec Review.

Comment 11

[Dietrich Ayala (:dietrich)]

Alex, do you have an ETA for a patch for this?

Comment 12

[Alexandre Poirot [:ochameau]]

Patch in progress, should be ready for tuesday/wednesday.

Comment 13

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 6376 - implement support for `custom-prompt` modals

Patrick, Could you take a quick overview on this patch and confirm that I've added a decent support for new 'custom-prompt' modals you addded throught bug 793644?

Ben, Here is a patch to fix reload issue, and eventually other modals that are of type 'custom-prompt'. I'm now trying to build integration unit test for this, but it isn't really clear if it is possible. Especially due to the need to serve a webpage...

Comment 14

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 681030 [details]
Pull request 6376 - implement support for `custom-prompt` modals

Etienne knows about custom dialog. Let's see if he can help with the review.

Comment 15

[Alexandre Poirot [:ochameau]]

Oh I just realized that I'm most likely missing some l10n work in my patch.
Can I still introduce new strings?
Should I just add these new strings to browser.en-US.properties or do I need to add them to all locales?

Comment 16

[Dietrich Ayala (:dietrich)]

Until 11/15.

Comment 17

[Ben Francis [:benfrancis]]

I thought browser was already string frozen? I'd like to know because I have some other string changes I need to make. Wasn't there a special process for late string changes?

Comment 18

[Alexandre Poirot [:ochameau]]

etienne, here is the testpage I used to play with this feature:
http://test.techno-barje.fr/form/

Comment 19

[Dietrich Ayala (:dietrich)]

(In reply to Ben Francis [:benfrancis] from comment #17)
> I thought browser was already string frozen? I'd like to know because I have
> some other string changes I need to make. Wasn't there a special process for
> late string changes?

Oops, yes you are correct. Contact Stas about late string changes. Please coordinate so he knows about all the late changes coming down the pipe.

Comment 20

[Etienne Segonzac (:etienne)]

Comment on attachment 681030 [details]
Pull request 6376 - implement support for `custom-prompt` modals

After Ben's review, please squash before landing :)

Comment 21

[Alexandre Poirot [:ochameau]]

Stas, This patch is about to introduce few strings that comes from mozilla-central:
https://github.com/mozilla-b2g/gaia/pull/6376/files#diff-2

( http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/global/commonDialogs.properties#12 )

Comment 22

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 681030 [details]
Pull request 6376 - implement support for `custom-prompt` modals

Etienne's review is enough for the prompt. Let's go ahead.

Comment 23

[Ben Francis [:benfrancis]]

Sorry it took me a while to get to this. 

The modal prompts code really needs a refactor, to use building blocks amongst other things, but we can address that later.

FWIW this patch is fine with me now that Etienne's suggested changes have been applied.

Thanks!

Comment 24

[Alexandre Poirot [:ochameau]]

Landed:
https://github.com/mozilla-b2g/gaia/commit/b700c4da43a5732ed1e5aa9c5cb6bcfca3937eb2

Comment 26

[Darren]

Verified as fixed in build 20121231070201.

Comment 27

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

https://moztrap.mozilla.org/manage/case/15078/