Closed Bug 297887 Opened 19 years ago Closed 19 years ago

Form values are not correct with bfcache enabled

Categories

(Core :: DOM: Navigation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.8beta4

People

(Reporter: markushuebner, Assigned: brettw)

References

()

Details

(Keywords: regression, Whiteboard: [no l10n impact])

Attachments

(3 files, 1 obsolete file)

Using trunk build 20050609 on win-xp pro. To reproduce try these steps: 1) Go to http://www.google.com and type in "mozilla" and hin enter. 2) Now type "browser" in the form-field and hit enter. When you go back one page now you still see "browser" in the form field instead of "mozilla".
Component: General → History: Session
Product: Mozilla Application Suite → Core
Assignee: general → nobody
QA Contact: general → history.session
See bug 293203, comment 21. I think that bfcache enabled prevents the onload event to get fired when going back into history (I think that's intentional, afaict from bug 274784). So this is an example where bfcache breaks an url because of th lack of onload-on-Back, see bug 274784, comment 5.
bryner has dibs on bfcache bugs, and can probably get content fixed here if that is necessary. /be
Assignee: nobody → bryner
fwiw, Safari and Opera behave the same way (neither of them fire the onload handler to the page when going back)
Alright, I'm slow today. What does 1.0.x do that 1.1a1 with fact-back enabled does not do? The google SERP page does document.gs.reset() from its onload; the main google page just sets focus. I don't see a difference in the contents of the input element in either page, following comment 0's steps (including unnumbered step 3, and even a step 4 that goes back again). I'm going to assume in the next sentence that I'm just confused above, and that we need to fix this bug somehow, with browser-only code changes: If we could discern that some pages need onload to be run, by monitoring what DOM mutations onload causes, perhaps we could do something. But in the better case, I would rather we not try to be too clever. Instead, we should work with other browser vendors to extend the DOM with the PageHide and PageShow events that bryner has proposed. Let's make progress in the DOM after years of stagnation, and make it possible for page authors to help us to comply with RFC 2616 (which BTW seems blissfully unaware of onload and dynamic content issues it raises). /be
The difference is that the search box at the top of the page always shows what you last typed into it, with fastback enabled. That means if you type in a different search term and submit, clicking back will show the different search term, not the term which generated the search results on that page.
Flags: blocking1.8b3?
OS: Windows XP → All
Hardware: PC → All
This is one of the rare cases where you *don't* want to return to the page as you left it. I've actually written pages like this myself although with a slight modification. I had a <select size=1> that was used for navigation. When the user selected something in the dropdown it navigated off to the selected page, but when going 'back' to the page you'd want to show the original value (which said something like "quick navigation") rather then the selected item. Of course we could use some heuristics and detect when the 'onload' reset some form and then reset it ourselvs on PageShow. But heuristics like that tend to just annoy more then help (MS word being a prime example) so I defenetly don't think it's the way to go. So the only resonable solution IMHO is evangelize. There is already the option to use the PageShow event and resetting when persisted=true. An even better long term solution would be to have an attribute on the form-control allowing the author to specify that a form or formfield should not be persited across page transitions. We would then not stick the formfield in the sessionhistory, and we'd reset it before bringing it back from bfcache.
sicking: Doesn't autocomplete="off" handle that case?
It should, but it does other things too which are not desiarable (i.e. it disables autocomplete :) )
Flags: blocking-aviary1.1?
Flags: blocking1.8b4+
Flags: blocking1.8b3?
Flags: blocking1.8b3-
Flags: blocking-aviary1.1?
Flags: blocking1.8b4?
Whiteboard: [no l10n impact]
I think we should probably leave the current behavior, however, it does seem like a good idea to give the author an easy way to reset the form when the page is restored. Keeping the bug open for that.
Target Milestone: --- → mozilla1.8beta4
Assignee: bryner → brettw
Attached a testcase for a future patch that will implement onPageShow for the body element to allow web page designers to detect this event.
Flags: blocking1.8b4?
Attached patch Patch to expose onPageShow/onPageHide (obsolete) (deleted) — Splinter Review
This patch exposes onPageShow and onPageHide via the body element and window.onPage[Show|Hide]. The case of the existing events was changed to all lower case, which is required for html attributes. I changed case for more things than were required for completeness, like when used as an argument to EqualsIgnoreCase.
Comment on attachment 190175 [details] [diff] [review] Patch to expose onPageShow/onPageHide >--- dom/public/coreEvents/nsIDOMPageTransitionListener.h 15 Jun 2005 23:52:37 -0000 1.1 >+++ dom/public/coreEvents/nsIDOMPageTransitionListener.h 22 Jul 2005 17:43:26 -0000 >@@ -47,12 +47,12 @@ class nsIDOMEvent; > */ > #define NS_IDOMPAGETRANSITIONLISTENER_IID \ > { 0x24f4d69f, 0x6b0c, 0x48a8, { 0xba, 0xb7, 0x12, 0x50, 0xcb, 0x5e, 0x48, 0x79 } } > > class nsIDOMPageTransitionListener : public nsIDOMEventListener { > public: > NS_DEFINE_STATIC_IID_ACCESSOR(NS_IDOMPAGETRANSITIONLISTENER_IID) > >- NS_IMETHOD PageShow(nsIDOMEvent* aEvent) = 0; >- NS_IMETHOD PageHide(nsIDOMEvent* aEvent) = 0; >+ NS_IMETHOD pageshow(nsIDOMEvent* aEvent) = 0; >+ NS_IMETHOD pagehide(nsIDOMEvent* aEvent) = 0; > }; > #endif // nsIDOMPageTransitionListener_h__ These should stay uppercase for consistency with the rest of the nsIDOM*Listener interfaces. >--- dom/src/base/nsDOMClassInfo.cpp 29 Jun 2005 03:51:43 -0000 1.286 >+++ dom/src/base/nsDOMClassInfo.cpp 22 Jul 2005 18:05:06 -0000 >@@ -1093,17 +1095,16 @@ jsval nsDOMClassInfo::sAdd_id > jsval nsDOMClassInfo::sAll_id = JSVAL_VOID; > jsval nsDOMClassInfo::sTags_id = JSVAL_VOID; > jsval nsDOMClassInfo::sAddEventListener_id= JSVAL_VOID; > > const JSClass *nsDOMClassInfo::sObjectClass = nsnull; > > PRBool nsDOMClassInfo::sDoSecurityCheckInAddProperty = PR_TRUE; > >- Stray whitespace change? Also, the changes to files under mozilla/toolkit need to be made to the corresponding files in mozilla/xpfe. Looks good otherwise.
Attachment #190175 - Flags: review+
New patch implementing bryner's suggestions.
Attachment #190175 - Attachment is obsolete: true
Comment on attachment 190416 [details] [diff] [review] Patch to expose onPageShow/onPageHide I assume that the changes to xpfe/components/shistory aren't supposed to be part of this patch. Looks good otherwise. Requesting approval to land this -- this patch makes it easier for page authors to interact with fastback by exposing "onpagehide" and "onpageshow" so that they can be used on the <body> tag like onload.
Attachment #190416 - Flags: review+
Attachment #190416 - Flags: approval1.8b4?
Attachment #190416 - Flags: approval1.8b4? → approval1.8b4+
Yes, the changes in xpfe/components/shistory are erroneous, sorry.
(In reply to comment #14) > Created an attachment (id=190416) [edit] > Patch to expose onPageShow/onPageHide > > New patch implementing bryner's suggestions. I checked in this patch. I'm also of the opinion that we might want to add an attribute that causes a form reset to happen automatically on pageshow, i.e. <form reset="true">
Could we use these onshowpage/onhidepage events to fix bug 277067?
The events aren't fired when switching tabs, only when a page is put into or restored from session history.
The <form reset="true"> idea is actually a different problem, because it's applicable whether or not fastback is enabled (see google.com search results, which reset the form to workaround form-value saving, independently of bfcache). So marking this fixed.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Well, but what about all sites out there where webmasters don't update their sites. There Mozilla will still show the wrong behaviour, or?
Webmasters would need to update their site for the reset="true" feature too.
*** Bug 361052 has been marked as a duplicate of this bug. ***
Component: History: Session → Document Navigation
QA Contact: history.session → docshell
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: