Form data is only remembered on back/forward for the first form in a page. I noticed this bug at http://www.cs.hmc.edu/~jruderma/s/, but it also happens at http://www.plastic.com/users.pl (if you're not logged in). 0314 - wfm 032903 - wfm 033109 - broken 040303 - broken 0419 - broken
Created attachment 80239 [details] testcase Steps to reproduce: 1. Fill in each form element, or just the last two. 2. Leave the page by submitting one of the forms or hitting the back button. 3. Return to the page. Result: what you entered in the second form is gone.
I am seeing this on linux as well trunk 2002042015 os -> all
OS: Windows 98 → All
Bug 134700 comment 3: also happens at http://www.mozilla.org/quality/help/bugzilla-helper.html, which has a search form at the top.
*** Bug 134700 has been marked as a duplicate of this bug. ***
Hmmm. We do some shenanigans in nsFrameManager::GenerateStateKey(). That may be the cause. Regardless, this is almost certainly me, and probably caused by the bug 108309 checkin. Nominating for nsbeta1
Assignee: radha → jkeiser
Keywords: nsbeta1 → nsbeta1+
Priority: -- → P1
Target Milestone: --- → mozilla1.0
*** Bug 140245 has been marked as a duplicate of this bug. ***
John, do you have an ETA on when you think you might have a patch for this?
I can have a patch for you right now that fixes the problem and is low-risk, but carries a 4-5% pageload hit. I will hopefully have a fix that does not take a pageload hit and still fixes the problem by Monday, but it will have higher risk. Let me know if you want the low-risk low-performance patch and I'll post it.
*** Bug 140697 has been marked as a duplicate of this bug. ***
BTW, I have thought up a low-risk way to do this for branch (I want to do it The Right Way for trunk). Specifically, save the current form #, or even immediately update the document.forms list (which would work better for dynamically added forms, which we barely support anyway). Patch tomorrow once I get away from meetings and such.
changing eta based on conversation with John.
Whiteboard: [adt1][m5+] → [adt1][m5+][eta 4/29]
Created attachment 81631 [details] [diff] [review] Patch The problem is that while we are parsing, document.forms has not been updated with all the forms yet. So when we try to restore, and it has not been updated yet, (the intent was) we generate the state key assuming that the form currently being parsed is the one the control is being added to. The way we did this hack was wrong, however, because when multiple forms are on the page, and *neither* has been placed in document.forms, we will use the index of the first form for controls in the second form. Calling FlushPendingNotifications(), which populates document.forms and does a lot of other stuff besides (like creating frames), fixes the problem but carries a 4-5% pageload penalty. This patch, which is WALLPAPER to allow us to work in 1.0 without the performance problems of FlushPendingNotifications or the high risk of the real fix. The real fix is to make FlushPendingNotifications not take so darn long (there are several ways of going about this) or even make certain content state update *immediately*.
Created attachment 81632 [details] [diff] [review] Patch (diff-u) diff -u, same patch
Attachment #81631 - Attachment is obsolete: true
Patch is awaiting sr=, a= and adt1.0.0+, all three mails sent, it's OK to hold your breath :) Won't happen until tomorrow morning though.
Whiteboard: [adt1][m5+][eta 4/29] → [adt1][m5+][eta 4/29][FIX]
Comment on attachment 81632 [details] [diff] [review] Patch (diff-u) sr=attinasi for the temporary patch. Actually, I like it better than the 'real' fix :) But then, I detest FlushPendingNotifications, and look forward to your ideas for making it faster...
Attachment #81632 - Flags: superreview+
adding adt1.0.0+. Please check this in as soon as possible and add the fixed1.0.0 keyword after getting drivers approval.
Keywords: adt1.0.0 → adt1.0.0+
Comment on attachment 81632 [details] [diff] [review] Patch (diff-u) firstname.lastname@example.org, please check into branch and trunk.
Attachment #81632 - Flags: approval+
Checked in to branch only. I'll ask Jesup what's up with the checkin to trunk thing.
Moving nsbeta1+ to nsbeta1- to get it off the nsbeta1 radar since a patch has already been checked into the 1.0.0 branch. This bug is left open to work on a fix for the trunk.
Keywords: nsbeta1+ → nsbeta1-
Is somebody moving towards getting this fixed on the trunk? It's a very annoying bug. While everybody's going on and on, ad nauseam, in Bug 46845 about whether or not form fields should reset upon page reload, this much more impactful data loss problem is being allowed to persist.
xah, are you not seeing this in 1.0? It should already be fixed there.
See also bug 58724, "should autofill password while page is loading, not onload". Could the same fix work for both bugs?
Same fix won't fix both bugs, but it has become apparent that the FlushPendingNotifications thing is going to take longer than anticipated, so I'm going after approval for this patch for 1.1.
a=asa (on behalf of drivers) for checkin to 1.1
Checked in to 1.1beta.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
Is it in the trunk as well?
Component: History: Session → Document Navigation
QA Contact: claudius → docshell
You need to log in before you can comment on or make changes to this bug.