Closed
Bug 301358
Opened 20 years ago
Closed 19 years ago
HTTP "refresh" doesn't load page with bfcache enabled
Categories
(Core :: DOM: HTML Parser, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla1.8beta4
People
(Reporter: shayne.anthony.jewers, Assigned: darin.moz)
References
Details
(Whiteboard: [serious bfcache regression] [1.8 Branch ETA 8/9])
Attachments
(2 files)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
Biesinger
:
review+
bryner
:
superreview+
benjamin
:
approval1.8b4+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050719 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050719 Firefox/1.0+
pages using meta refresh dont seem to load the page and i have to reload the
page for it to come up
Reproducible: Always
Steps to Reproduce:
1) go to www.thedailypos.org/forum/
2) login with username: Name password: password
3) click the profile button
4) select "Forum Profile Information"
5) Click "Change Info"
Actual Results:
Blank page comes up, though reloading the page makes it come up
Expected Results:
the pages loads
Updated•20 years ago
|
Blocks: blazinglyfastback
Reporter | ||
Comment 1•20 years ago
|
||
should also note, some people say this also happens when logging in and logging
out
Comment 2•20 years ago
|
||
I see it also happen that way, get twice a blank page.
With browser.sessionhistory.max_viewers to 0 I go to the pages rightaway.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050719
Firefox/1.0+ ID:2005071906
Comment 3•20 years ago
|
||
Please check when this regressed. This may be the same issue as bug 300944 (so
the regression period should be the same then).
Comment 4•20 years ago
|
||
This bug is only triggered when the refresh is in the HTTP header and *not* in a
"meta http-equiv=refresh" in the document itself. Time does not seem to matter.
See testcase
http://maxradi.us/post/bugzilla/refresh/
which can not be attached for obvious reasons. Here's the CGI:
echo "refresh: 2;url=foo.html"
echo
echo This is the refresh page, it should go away after 2 secs
For comparison, here's a simple meta refresh page which works:
http://maxradi.us/post/bugzilla/refresh/meta.html
Assignee: parser → bryner
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Meta refresh doesent load page with bfcache enabled → HTTP "refresh" doesen't load page with bfcache enabled
Comment 6•20 years ago
|
||
Regressed between ID 2005062806 and ID 2005062823.
Updated•20 years ago
|
Flags: blocking1.8b4?
Assignee | ||
Comment 7•20 years ago
|
||
There's probably a bug in the suspending and resuming of refresh timers. HTTP
refresh headers are inserted into the refresh timer list from nsDocument.cpp:
http://lxr.mozilla.org/mozilla/source/content/base/src/nsDocument.cpp#4540
That code runs from nsDocument::StartDocumentLoad.
whereas META refresh timers are inserted from the nsContentSink's call to
nsDocument's SetHeaderData method:
http://lxr.mozilla.org/mozilla/source/content/base/src/nsContentSink.cpp#313
so, it's possible that the docshell would learn about the refresh timers at
different times...
Updated•20 years ago
|
Flags: blocking1.8b4? → blocking1.8b4+
Assignee | ||
Updated•20 years ago
|
Severity: normal → major
Status: NEW → ASSIGNED
Priority: -- → P1
Assignee | ||
Comment 9•20 years ago
|
||
So, this bug appears to be fixed in my debug CVS build from today. However, my
trunk build from yesterday does exhibit the problem. Investigating...
Assignee | ||
Comment 10•20 years ago
|
||
(In reply to comment #9)
> So, this bug appears to be fixed in my debug CVS build from today. However, my
> trunk build from yesterday does exhibit the problem. Investigating...
Whoops.. ignore that. It helps to have a clean profile that doesn't have a
certain preference set to 0 :-/
Updated•20 years ago
|
Summary: HTTP "refresh" doesen't load page with bfcache enabled → HTTP "refresh" doesn't load page with bfcache enabled
Updated•20 years ago
|
Whiteboard: [serious bfcache regression]
Assignee | ||
Updated•19 years ago
|
Whiteboard: [serious bfcache regression] → [serious bfcache regression] [1.8 Branch ETA 8/9]
Assignee | ||
Updated•19 years ago
|
Blocks: branching1.8
Assignee | ||
Updated•19 years ago
|
No longer blocks: branching1.8
Assignee | ||
Comment 11•19 years ago
|
||
OK, so bryner and I devised a pretty straightforward solution to this bug. We
realized along the way that nsDocShell::LoadURI ends up canceling all timers,
which means that clicking on a link that results in a download kills timers.
We found that IE does the same, so this patch doesn't fix that behavior.
However, when you navigate back to such a page, we will restore the timers
(also similar to how IE works and Moz works w/o fastback). The trick to this
patch is that we change the cancelation in Stop to a suspend, and we save off
mRefreshURIList. This allows us to get access to the list inside CaptureState.
The only remaining trick then is to clear the saved copy of mRefreshURIList
inside CreateContentViewer and its peer RestoreFromHistory.
Attachment #192315 -
Flags: superreview?(bryner)
Attachment #192315 -
Flags: review?(cbiesinger)
Assignee | ||
Comment 12•19 years ago
|
||
This patch also makes meta refresh work properly when the user navigates back to
a restored page. Previously, we were not saving any meta refreshes for the page
when leaving it, and this patch fixes that as well.
Comment 13•19 years ago
|
||
Comment on attachment 192315 [details] [diff] [review]
v1 patch
+ mSavedRefreshURIList = mRefreshURIList;
+ mRefreshURIList = nsnull;
.swap() ?
So.. if a load ends up in a different docshell (e.g. it is a download), then
shouldn't the timers be resumed?
Assignee | ||
Comment 14•19 years ago
|
||
> + mSavedRefreshURIList = mRefreshURIList;
> + mRefreshURIList = nsnull;
>
> .swap() ?
Yeah, good idea :)
> So.. if a load ends up in a different docshell (e.g. it is a download), then
> shouldn't the timers be resumed?
Yeah, that is what I would think too, but as I commented above that bug exists
today, and moreover it appears to exist in IE as well. So, I think we can
separate the issues. We don't need to solve that bug here.
Comment 15•19 years ago
|
||
Comment on attachment 192315 [details] [diff] [review]
v1 patch
ok, r=biesi with swap() used
Attachment #192315 -
Flags: review?(cbiesinger) → review+
Assignee | ||
Comment 16•19 years ago
|
||
OK. I'm still nulling out mRefreshURIList after the .swap call though since I
don't trust that mSavedRefreshURIList is null at that point.
Updated•19 years ago
|
Blocks: branching1.8
Comment 17•19 years ago
|
||
Reminder that we are closing the tree tonight at 11:59PM PDT for branch. Let us
know if this isn't going to make it.
Updated•19 years ago
|
Attachment #192315 -
Flags: superreview?(bryner) → superreview+
Assignee | ||
Updated•19 years ago
|
Attachment #192315 -
Flags: approval1.8b4?
Updated•19 years ago
|
Attachment #192315 -
Flags: approval1.8b4? → approval1.8b4+
Assignee | ||
Comment 18•19 years ago
|
||
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 19•19 years ago
|
||
This caused bug 312769 -- when destroying a docshell (window close, subframe,
etc) we call Stop(), and there will be no more content viewer creations. So with
this patch we never broke the timer -> docshell -> supports array -> timer
cycles in those cases.
You need to log in
before you can comment on or make changes to this bug.
Description
•