299008 - Going back from a XUL error page does not restore form data

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P1)

Description

[Caleb]

It seems that if you type something in a text field (this probably affects other
form elements aswell), go to an invalid website, and then press back and the
text is lost!

This seems to affect both XML parser errors, and XUL error pages.

To reproduce:
1. Open this page https://bugzilla.mozilla.org/show_bug.cgi?id=295732
2. Type anything in the "Additional Comments:" textarea
3. Type blagasfasdfasdfasdf.com in the address bar and press Enter
4. Press back, and the text is gone!

Note: Step 3 results in an XML error, but it should result in a XUL error page
(this is a "bug" with the XUL error pages - missing description of an error or
such).

Comment 1

[kick rocks]

(In reply to comment #0)
> It seems that if you type something in a text field (this probably affects other
> form elements aswell), go to an invalid website, and then press back and the
> text is lost!
> 
> This seems to affect both XML parser errors, and XUL error pages.
> 
> To reproduce:
> 1. Open this page https://bugzilla.mozilla.org/show_bug.cgi?id=295732
> 2. Type anything in the "Additional Comments:" textarea
> 3. Type blagasfasdfasdfasdf.com in the address bar and press Enter
> 4. Press back, and the text is gone!
> 
> Note: Step 3 results in an XML error, but it should result in a XUL error page
> (this is a "bug" with the XUL error pages - missing description of an error or
> such).

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050628 Maimedbat
Firefox/1.0+

Same result when tested on this build.

Comment 2

[Caleb]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050628
Firefox/1.0+ ID:2005062808

Testing with a new build that fixes the XML error (now a XUL error page shows up
at step 3).

Note: Bfcache/fastback is _DISABLED_!

Comment 3

[:Gavin Sharp [email: [e-mail]]]

I see the behavior described in comment 0 as far back as 20050201 on the trunk.
Will test further.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Regression range: Between 2005-01-30 08:00 and 2005-01-31 07:00

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-01-30+08%3A00&maxdate=2005-01-31+07%3A00&cvsroot=%2Fcvsroot

Bug 157004 fixed the fact that you had to reload an error page to go back to the
previous page using the back button, but it apparently caused this bug.

Comment 5

[Benjamin Smedberg]

biesi, can you take a look please?

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

I do not see this on a page with XML parse errors, only on an error page.
Testcase: type something in this bug's additional comment field, load
data:text/xml,asdf, click back -- text is still there.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

What's going on is this: Embed saves the form data on the docshell's current
mOSHE.
Embed is called when the document starts loading, which is some time after
LoadErrorPage.
But LoadErrorPage overwrote mOSHE with mLSHE, which means that the form data
was stored on the error page's SH entry.

With this patch, mOSHE gets its new value in Embed.

Comment 8

[Johnny Stenback (:jst)]

Comment on attachment 187582 [details] [diff] [review]
patch

r=jst, but I'd love to have either bz or darin look at this too.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 187582 [details] [diff] [review]
patch

sr=bzbarsky if everything (page titles, etc) is still happy.

Comment 10

[Caleb]

After applying the patch:

 * Going back from the XUL error page restores the title, etc..
 * The "Try Again" button in the error page re-loads the previous page, and not
the current operation.

Comment 11

[Caleb]

(In reply to comment #10)
> After applying the patch:
> 
>  * Going back from the XUL error page restores the title, etc..
>  * The "Try Again" button in the error page re-loads the previous page, and not
> the current operation.
> 

Oh, forgot to mention that it DOES fix this bug.

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

I think the page title comment was more about shistory entries... that the title
bar etc show the right title is really no surprise, that uses different mechanisms.

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 187582 [details] [diff] [review]
patch

hm, yeah. try again does reload the wrong page...

history titles are correct though.

Odd... try again and reload now do different things...

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: fix try again

The mLSHE that referred to the real page load was cleared in InternalLoad; this
patch makes it so that it doesn't get cleared and that thus mOSHE will
eventually refer to the right page.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 188111 [details] [diff] [review]
fix try again

sr=bzbarsky, but file a followup bug on checking out geneneral interaction of
bfcache and error pages, ok?  I suspect they don't play nice with each other.

Comment 16

[Christian :Biesinger (don't email me, ping me on IRC)]

filed bug 299547

Comment 17

[Caleb]

(In reply to comment #14)
> Created an attachment (id=188111) [edit]
> fix try again

After applying this patch:

1. Go to http://www.betanews.com and wait for it to completely load
2. Type anything in the "search" box on the top-left part of the page
3. Go to http://asjfdaskfhaskjhfkahfhkajhfaksjdfh.com or just show a xul error page
4. Go back

Results:

* With bfcache: the form elemnt is emptied (dataloss)
* Without bfcache: the form element is kept intact

I'd like a confirmation on this!

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

I'd like to leave bfcache issues to bug 299547. it's not like I know the code..

Comment 19

[Johnny Stenback (:jst)]

Comment on attachment 188111 [details] [diff] [review]
fix try again

r=jst

Comment 20

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 188111 [details] [diff] [review]
fix try again

this patch makes sure that form info is stored on the right history entry, so
that it can successfully be restored.

Comment 21

[Benjamin Smedberg]

Comment on attachment 188111 [details] [diff] [review]
fix try again

a=bsmedberg for expeditious landing. If this doesn't make the nightlies
tomorrow morning, please check with me or cbeard.

Comment 22

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in docshell/base/nsDocShell.cpp;
/cvsroot/mozilla/docshell/base/nsDocShell.cpp,v  <--  nsDocShell.cpp
new revision: 1.697; previous revision: 1.696
done


should be in time for the nightlies, I assume.