823467 - Should private browsing mode in docshell disable bfcache?

Status: REOPENED

(Core :: DOM: Navigation, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

Filing this just that I don't forget the issue.
Feel free to mark INVALID if this isn't an issue.

As far as I know we may change private browsing mode of a docshell.
What happens if: We're first in pb and load a page. Then load another page and decide to
un-pb, and then go back. Do we let that page, which was loaded in pb mode to be
re-activated in a docshell which isn't in pb mode anymore?

Comment 1

[:Gavin Sharp [email: [e-mail]]]

I thought we weren't trying to support changes in a docShell's PB state after creation.

Comment 2

[Josh Matthews [:jdm]]

Yes, I suspect this is a problem that we're not going to bother dealing with.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

So we let page which has been loaded in pb mode to be re-activated in non-pb mode?

Currently the code in no way prevents changing the state. If we don't want to let state to be
changed, better to throw an exception if one tries to do it.

Comment 4

[(no longer active)]

Which code are you talking about?  The bf-cache code or the docshell's private flag?

Also, any reason why we should keep this bug hidden?

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Docshell's code. If we don't support changing pb flag after setting it, docshell should
actually throw if someone tries to re-set it.

And yes, I guess we can open this.

Comment 6

[(no longer active)]

(In reply to comment #5)
> Docshell's code. If we don't support changing pb flag after setting it,
> docshell should
> actually throw if someone tries to re-set it.

I'm not sure if there's a very easy way to detect whether the flag is being set on a "new" docshell or on an "old" docshell...

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

By default the flag is not set, right? So after it has been set once, never allow changes.
That would at least prevent the case when a page is first loaded in pb mode and then
navigated back from shistory to docshell which isn't in pb mode anymore.

Comment 8

[(no longer active)]

Is this warning not enough? http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#2029

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Uh, that is bizarre. Why do we have an attribute in nsIDocshell which warns always when set?

Comment 10

[(no longer active)]

(In reply to comment #9)
> Uh, that is bizarre. Why do we have an attribute in nsIDocshell which warns
> always when set?

That is the only way that external code can set this attribute.  Copies internal to nsDocShell.cpp should all be safe.