Closed Bug 74639 Opened 24 years ago Closed 24 years ago

Going back to forms loses form data.

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla0.9

People

(Reporter: bzbarsky, Assigned: pollmann)

References

Details

(Keywords: dataloss, regression, Whiteboard: important to mozilla0.9)

Attachments

(6 files)

This is a consequence of bug 73490 being fixed. If I fill out a form at say http://bugzilla.mozilla.org/show_bug.cgi?id=73490, then click on "Vote for this bug" and then hit "back", the form data should not be cleared.... Currently it is (linux build 2001-04-02-05).
Keywords: dataloss, regression
I think this is more of a history issue, rather than a cache issue.
Assignee: gordon → radha
Component: Networking: Cache → History: Session
Keywords: nsbeta1, nsCatFood
Makes bugzilla hard to use -- I keep losing data because of mistyping someone's email address or forgetting a component and getting the error page, and losing my changes when I go back.
*** Bug 72701 has been marked as a duplicate of this bug. ***
I'm seeing some strange behavior... When I go to yahoo.com, type in a search term, then click back two things happen. First, the page lays out and the history state is restored correctly, then the entire page goes blank, and all of the history state is blanked out. I don't know why the page is blanking, but this may be another symptom of the underlying problem?
platform all/all. dupe bug 72701 has bugzilla example. radha, the dupe's target milestone was set to 0.9.1
OS: Linux → All
QA Contact: tever → claudius
Hardware: PC → All
I think we want to fix this for mozilla0.9. Radha, can a fix be developed and tested/reviewed in the next few days? We'd take it into the frozen trunk or the 0.9 branch, once the trunk re-opens for 0.9.1. Can you say more about what broke and what's needed to fix it? /be
Keywords: mozilla0.9
Whiteboard: important to mozilla0.9
Looking at the bugzilla example, the layoutHistoryState is set on the new PresShell by docshell. The PresShell call returns success, This should take care of putting the form values back on the page. I don't see a reload or a re-rendering happening on the bugzilla page. Agree that this needs to be fixed ASAP. Shall try to get it in for 0.9.
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla0.9
cc'ing jst, as I understand he may have some clue on what broke this feature. Bugzilla form elements are working fine on my 4/9 builds, but not on 4/16 builds
Both yahoo.com and bugzilla pages have badly nested forms, which causes the form and all of it's contents to be demoted from a container to a leaf element. The frames for form controls (text inputs, ...) get destroyed and recreated, which causes us to loose the information. When they get recreated, the information *should* be restored, but isn't for some reason. (It is possible that this never worked, but a recent change, e.g. improved cache, could have made the problem more obvious and on more pages. If the page comes in faster or in larger chunks, form demotion could cause larger blocks to get demoted / more state to be lost). I'm also looking at this and will update the bug report if I can make heads or tails of what is going on.
Attached file reduced test case (deleted) —
Can't help yahoo.com, but I'm cc'ing mozilla-staffers who can help ensure that a bug against bugzilla for spewing unbalanced forms is on file. /be
This patch fixes bugzilla, yahoo, and the reduced test case. Do we also want to put CaptureStateFor before the second DeletingFrameSubtree in nsCSSFrameConstructor::ContentRemoved? CC'ing Nisheeth for insight on the performance impact of this change.
I did some clock-watch performance testing of this change. In the bugzilla query page, when a new component is selected, this code gets called a *lot* (thousands of times). Select "Browser component: Before: 15 seconds After: 15 seconds Select "Browser Localization" component: Before: 9 seconds After: 9 seconds Seems that if it does have a performance impact, it is drowned out by the work done to destroy and recreate the frames themselves.
Turns out we can't use this patch because it may cause bad things to happen to XUL tree widget performance. I'm working on another patch that is specific to SinkContext::DemoteContainer, which I'll attach as soon as I get it compiling...
I think it is appropriate for pollmann to own this bug right now.
Assignee: radha → pollmann
Status: ASSIGNED → NEW
Accepting...
Status: NEW → ASSIGNED
Bug 76714 is the Bugzilla bug that exposes this problem, although I was unable to consistently reproduce it with 2001-04-02-05 and 2001-04-18-08. Sometimes my form data was lost, sometimes it wasn't, and sometimes the data that appeared was from the second-to-last time I filled out the form rather than the last time.
myk@mozilla.org wrote: > Sometimes my form data was lost, sometimes it wasn't, and sometimes the data > that appeared was from the second-to-last time I filled out the form rather > than the last time. I've seen this last thing (sometimes the data is from the second-to-last time) too, very rarely. Is it a separate bug? /be
That last behavior is definitely a separate bug. Radha, is one already filed on that?
r=nisheeth@netscape.com, sr=jst@netscape.com Ready to go, just waiting for approval.
a= asa@mozilla.org for checkin to 0.9
Patch 4 greatly exacerbates a problem where, occasionally, going to a page would entirely blank out many or all controls. (All radios deselected, list box options deselected, first option of combo selected) The problem turned out to be frames happily storing this blank state into the LayoutHistoryState if they are requested to save their state before they are initialized. Patch 4 would cause this to happen quite frequently, as frames are commonly not yet initialized when the form demotion happens. The solution in patch 5 is to prevent this from happening by having frames *not* save state until they are initialized. Rerequesting r,sr,a
sr=jst, fixing bugzilla to not feed us with ill-nested tags would be a good thing to do, is there a bug on that?
Yes, there is. Bug 76714. It even has a patch on it already. However, Bugzilla's in the middle of a release cycle right now so there's a freeze on checkins pending finalization of the docs. So it may be a week or two before it gets checked in.
Nisheeth says: r=nisheeth@netscape.com, just waiting for a=...
asa was ready to a= the earlier patch. I'll a= this one, beating him to it, if you coders and reviewers are all happy. Get it in. /be
Fix checked in. To verify: 1) Go to http://www.yahoo.com, enter a search term, press Enter, click back button - your search term should still be there. 2) Open the reduced test case, enter a string. Type another URL in the url bar, click back - term should still be there. -OR- 3) Go to bug query page or a bug report, make some changes, go somewhere else, click back - the changes you made should still be there. Thanks!
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
this mostly works BUT try these steps: Go to yahoo.com 1. enter a search term 2. now click any link, I clicked 'shopping'. 3. Click back --> no search term. This should work just like the bugzilla testcase. For fun continue with these steps: 4. Click forward again. 5. Click back again - now the search button is blank? What up with that? Continue with these steps or them instead of 4 and 5: Alternate 4. type a search string. Alternate 5. Click forward again Alternate 6. Click back. Now your search string is the label on the search button instead of 'Search' like yahoo intended. *I couldn't break the reduced testcase or the bugzilla case, but this bug is reopened just for failing my steps 1,2, and 3. The other stuff is just freakish related behavior I felt I should document since I figured out how to reproduce it.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Seeing what claudius is seeing as well.
The blank button (and probably the blanked state on the same page) sounds like an off-by-one error with the content ID. This is a symptom of our whole system of keying the session history state by the content ID being flawed. It's a known problem that's been around since forever. I'll open a new bug on making this smarter or "fuzzier" in some way. In the meantime, since the majority case that was broken is now fixed, re-marking this as fixed - will attach bugid of new bug...
Status: REOPENED → RESOLVED
Closed: 24 years ago24 years ago
Resolution: --- → FIXED
New issue is bug 77834.
marking this VERIFIED Fixed based on my earlier testing since remaining issues are addressed in a different bug (bug 77834).
Status: RESOLVED → VERIFIED
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.

Attachment

General

Creator:
Created:
Updated:
Size: