Closed Bug 305236 Opened 19 years ago Closed 19 years ago

The bfcache stores too many things in the cache

Categories

(Core :: DOM: Navigation, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: mrbkap, Assigned: mrbkap)

References

Details

(Keywords: fixed1.8)

Attachments

(1 file, 2 obsolete files)

Currently, when we have a window.open(url) call, we first load an about:blank
page and then load the actual page in that same docshell/window. While the
about:blank page doesn't make it into session history (i.e., you can't click
back to go to it), the bfcache *does* store the about:blank page in the cache.
This confuses us, since this is a case where we want to reuse the (about:blank)
inner window.

The solution is to not put the about:blank document in the bfcache.wodniw ren
Attached patch Fix (obsolete) — Splinter Review
With this fix, we refuse to try to store the current page in the bfcache if
we're going to reuse the inner window. Note that in these cases, mOSHE is never
actually put into the session history, so storing the content viewer really is
wrong.
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
Attachment #193227 - Flags: superreview?(jst)
Attachment #193227 - Flags: review?(bryner)
Comment on attachment 193227 [details] [diff] [review]
Fix

jst and I talked about this and WouldReuseInnerWindow is going to take a
document instead of a URI.
Attachment #193227 - Attachment is obsolete: true
Attachment #193227 - Flags: superreview?(jst)
Attachment #193227 - Flags: review?(bryner)
Attachment #193234 - Flags: superreview?(jst)
Attachment #193234 - Flags: review?(bryner)
Comment on attachment 193234 [details] [diff] [review]
pass the document instead of the URI

I'm sorry, I'm rushing too much. aRemoveEventListeners doesn't seem to get set
correctly with this patch. I propose to do: aRemoveEventListeners =
!reUseInnerWindow; though we should probably get rid of that parameter
altogether.
Attachment #193234 - Attachment is obsolete: true
Attachment #193234 - Flags: superreview?(jst)
Attachment #193234 - Flags: review?(bryner)
Attachment #193236 - Flags: superreview?(jst)
Attachment #193236 - Flags: review?(bryner)
Nominating for blocking, since this blocks a blocking 1.8b4+ bug.
Flags: blocking1.8b4?
Attachment #193236 - Flags: review?(bryner) → review+
Flags: blocking1.8b4? → blocking1.8b4+
Comment on attachment 193236 [details] [diff] [review]
set aRemoveEventHandlers too

sr=jst
Attachment #193236 - Flags: superreview?(jst) → superreview+
Fix checked into trunk. I'll check this into the branch tomorrow.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Whiteboard: [needs branch checkin]
Comment on attachment 193236 [details] [diff] [review]
set aRemoveEventHandlers too

This needs to be checked in on the branch.
Attachment #193236 - Flags: approval1.8b4?
Attachment #193236 - Flags: approval1.8b4? → approval1.8b4+
Checked into MOZILLA_1_8_BRANCH.
Keywords: fixed1.8
Whiteboard: [needs branch checkin]
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

Created:
Updated:
Size: