487695 - Decide on localStorage behavior in session-only cookies or private-browsing mode

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Honza Bambas (:mayhemer)]

In bug 486654 comment 11 started a discussion how should localStorage behave when cookies were in a session-only mode for a host. 

It's at this moment unclear, there are already some threads about it: http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2009-April/019238.html.

This bug should point out that how localStorage behaves now is not really correct and care to fix it.

Comment 1

[Daniel Veditz [:dveditz]]

Note that whatwg discussion was started talking about private-browsing mode, not the session-only cookie setting. Best case we can treat both the same, but we need to keep both in mind or we might end up with a solution that favors one over the other.

Comment 2

[Honza Bambas (:mayhemer)]

(In reply to comment #14)
> I'm confused about what you think I agree with, and I'm not entirely sure what
> returning an error at that point in C++ means to a caller from web content (is
> it an exception? or turned into something else along the way?).
> 

Returning an error in c++ (return NS_ERROR_DOM_QUOTA;) will raise an exception in JS, not sure what kind, though.

> Our overriding goal should be to have a compatible implementation--if we didn't
> care we could stick with globalStorage!

Why exactly we should stick with globalStorage?

> That means we're going to have to hash
> out the spec with WHATWG and that's not going to make any immanent code-freeze.
> But I sure hope the "another release" we'll fix this for is before 3.5 final!
> 

Agree, there is already the thread where it's discussed.

> What I don't like about our current scheme is that it is _not_ just
> "localStorage but not written to disk". If you had a multi-page web-app its
> interaction with "localStorage" would be complete different if cookies were set
> to session-only, and the app would be better off knowing that's the case so
> they could fall back to sessionStorage (or server-side storage).
> 
> I don't mind Brady's option 1 or 2 so much, unlike him. At least not in the
> "cookies are blocked" case, maybe his concern is that it could be a hint that
> the user is in private-browsing mode. I might slightly prefer removing the
> localStorage object entirely because it reinforces object detection which sites
> ought to be doing to handle browsers that don't support it. But that means we
> can't throw an exception when the user tries to get the object, it's just not
> there.
> 
> But that means the thing that adds localStorage to the DOM has to know that
> logic, it can't be something inside the localStorage code :-(. 

Yes, to remove localStorage from DOM would be the best short-term solution but also not that simple to implement. I don't think that any of these suggestions will really be a solution to this bug.

> So second best
> an effective quota of 0 seems to fit the bill in my mind. I'd have clear() and
> removeItem() silently fail as Brady suggested.
> 

It's just a risk that some web apps won't handle this case very well. Other con is that user might get annoying prompts about low quota when handled correctly by an app.

> Brady prefers his option 3, but that's not what we were doing. If we did that
> and had a fully workable in-memory localStorage object I could live with it,
> but ours was crippled. Kind of the worst of both worlds IMHO.
> 

True, it's based on globalStorage code that doesn't count with this. There are some arguments against being read-only because of potential data loss, but it should be well understood by user he's in an anonymous mode and all data will be deleted after exit.

> His option 4 sounds hard/slow and requires us to get option 3 working correctly
> first. I like his option 5 for private browsing and for the session-only
> cookies state it collapses to option 2 since nothing ever got stored there.
> 

I'm not sure if we want to expose existing data in the storage as we already don't expose cookies. Also Chrome is using whole new profile (as we also should!) that prevents any cookies to be exposed, but to decide on this is beyond this bug.

Comment 3

[Honza Bambas (:mayhemer)]

We should block on this, it's important for the final release.

Comment 4

[Johnny Stenback (:jst)]

Absolutely, we need *a* resolution to this before final.

Comment 5

[Daniel Veditz [:dveditz]]

(In reply to comment #2)
> > Our overriding goal should be to have a compatible implementation--if we
> > didn't care we could stick with globalStorage!
> 
> Why exactly we should stick with globalStorage?

globalStorage is a completely equivalent feature that we have already implemented. The only compelling reason to implement localStorage is to match the evolution of the HTML 5 spec so there is cross-browser compatibility. If we don't end up compatible we lose and the effort was wasted -- web authors still have to write if browserA/else browser B code which they can already do with globalStorage.

Note that globalStorage was a compliant implementation at one point, we've been burned by spec drift on this once already. If the spec is ambiguous or broken we should get it fixed rather than hope it eventually sees things our way.

> user might get annoying prompts about low quota when handled correctly
> by an app.

That's a fair counter-argument, but doesn't seem to leave us any good choices.

Comment 6

[Nochum Sossonko [:Natch]]

Part of the private browsing requirement, iirc, is that content should not be able to tell when a user is in pb mode or not. If storage throws (or for that matter if it behaves any differently) it might make it obvious that the user is in pb mode...

Comment 7

[Honza Bambas (:mayhemer)]

I absolutely agree with this and take it as a main goal/condition for the solution. However, in the thread I refer here are voices to make it possible to detect, even have an API for that, not sure how much expert those voices are. I'm personally against that, thought.

Comment 8

[(no longer active)]

(In reply to comment #7)
> I absolutely agree with this and take it as a main goal/condition for the
> solution. However, in the thread I refer here are voices to make it possible to
> detect, even have an API for that, not sure how much expert those voices are.
> I'm personally against that, thought.

This would mean an API to invade user's privacy.  There *are* ways for a website to guess if the user is in private browsing mode right now, but they're usually hacks which are either too complex to be considered to protect against, or out of our control (like for example using Flash cookies).  But part of what we considered in the implementation of private browsing was to make it extremely difficult for websites to tell this (and not promise anything to the users in that regard, because we're not providing 100% protection at any rate.)

We didn't take a throwing approach for DOM Storage partly because of this reason (see bug 463000), and I highly suggest against this path here.

I must say that I have not read that thread completely, but my suggestion is treating the storage data as session-only (just like cookies.)  Is there any reason such an approach won't be acceptable?

Comment 9

[Honza Bambas (:mayhemer)]

(In reply to comment #8)
> I must say that I have not read that thread completely, but my suggestion is
> treating the storage data as session-only (just like cookies.)  Is there any
> reason such an approach won't be acceptable?

There is no technical reason against, it's very close to a way of a fresh profile. localStorage implementation based on the common dom storage code is not prepared for it at the moment, but it's of course doable, there is already a related bug. We could kill two flies by one shot. 

There are only concerns about potential data loss. But, as I'm think of it, take an (offline) web application that stores some user data locally. You probably have to login to an account to work with it, i.e. to provide cookies. In PB mode we do not provide them, then you cannot work with that web app and therefor you cannot loose data. Unless you login accidentally while in PB mode, but it's obvious user mistake. We can give more UI to prevent this, like "you are in PB mode", "web app is storing and data will be lost" or so.

Comment 10

[(no longer active)]

(In reply to comment #9)
> There are only concerns about potential data loss.

What kind of data loss?

> But, as I'm think of it,
> take an (offline) web application that stores some user data locally. You
> probably have to login to an account to work with it, i.e. to provide cookies.
> In PB mode we do not provide them, then you cannot work with that web app and
> therefor you cannot loose data. Unless you login accidentally while in PB mode,
> but it's obvious user mistake. We can give more UI to prevent this, like "you
> are in PB mode", "web app is storing and data will be lost" or so.

I'd say that out current UI is more than enough (we show a full page and close down the whole session, if users don't notice that, they won't probably notice it even if we shout at them :-) ).  But I guess any web app using local storage should be prepared for the "no data" case, because the first time that someone works with their app, that happens.  So, even when a user logs in to their system, it should behave like it's the first time that the app is being used.  I don't understand how that can be interpreted as "data loss".

If the concern is the data which is saved during the private browsing mode, we make it clear that no data which can be used to track users' behavior is ever stored in private browsing mode, so that is in fact compatible with the way the rest of the browser works in that mode.

Comment 11

[Honza Bambas (:mayhemer)]

OK, I probably was not clear. Data loss means that when I use an offline web app (for making some notes or whatever) while in PB mode, the data I created is not persistently saved. After I exit PB mode, all is lost if not stored to the server but just intended to be kept locally (in localStorage or whatever storage).

It's not a problem that localStorage is empty at the start, I never talked about that case.

We make clear to user, that tracking data is not stored, but many users don't understand what it is. And yet fewer users don't consider their own personal data (like notes or something) as a tracking data. They will consider it as something that will be stored and never lost. But it will be lost on PB mode exit.

I'm not trying to argue against switching localStorage to session only mode, I'm just trying to explain some people's concerns.

Comment 12

[(no longer active)]

(In reply to comment #11)
> OK, I probably was not clear. Data loss means that when I use an offline web
> app (for making some notes or whatever) while in PB mode, the data I created is
> not persistently saved. After I exit PB mode, all is lost if not stored to the
> server but just intended to be kept locally (in localStorage or whatever
> storage).

Yes.  This is what I was talking about in the last paragraph of comment 10.

> It's not a problem that localStorage is empty at the start, I never talked
> about that case.

I see.  I was just speculating the different cases.  :-)

> We make clear to user, that tracking data is not stored, but many users don't
> understand what it is. And yet fewer users don't consider their own personal
> data (like notes or something) as a tracking data. They will consider it as
> something that will be stored and never lost. But it will be lost on PB mode
> exit.

Hmm, CCing Alex to see if he thinks something should be changed in the UI for this purpose.  Maybe an info bar when a page uses localStorage in private browsing mode?
 
> I'm not trying to argue against switching localStorage to session only mode,
> I'm just trying to explain some people's concerns.

Yes, I understand.  Thanks for bringing up these points.

Comment 13

[Alex Faaborg [:faaborg] (Firefox UX)]

>Hmm, CCing Alex to see if he thinks something should be changed in the UI for
>this purpose.  Maybe an info bar when a page uses localStorage in private
>browsing mode?

This infobar would make sense if the user just explicitly did something, like tried to save a draft of an email message, but wouldn't we also end up displaying the bar when the user wasn't necessarily expecting it, and they wouldn't understand why it was displayed, or what it means?

So my initial reaction is that it seems like it would be better not to solve this problem at a user interface level, but as people have pointed out one of the design considerations of private browsing mode was that sites shouldn't be able to tell if you are in private browsing mode or not.

I'm going to punt this one up to mconnor for a decision.

Comment 14

[Honza Bambas (:mayhemer)]

Attachment: wip 1

mSessionOnly member of nsDOMStorage is set when cookies are in session-only mode OR we are PB mode. I added this condition to nsDOMStorage::CanUseStorage. Then, when checking access to localStorage (only localStorage) with setItem or getItem we throw a security error when being session only (addresses this bug).

Comment 15

[Honza Bambas (:mayhemer)]

Ups, wrong bug, belongs to 488446.

Comment 16

[Honza Bambas (:mayhemer)]

Proposal one:
Goals for PB mode:
1. Give content no chance to detect we are in private browsing mode
2. Provide no existing data that are stored in the persistent database already
3. Ensure for 100% that after we return from PB mode all is lost/deleted, no trace left

Goals for while cookies are in session-only mode, for a site or whole browsing:
1. Give content no chance to detect cookies are in such mode
2. Do provide existing data
3. Do not store any changes to the persistent database but keep them for a browser lifetime (cookie lifetime)


Solution:
Have an in-memory database (just the same API as used for db access but implemented as a hashtable). Use it when in session-only cookies mode pre-filled by data from the persistent database. Use it when in PB mode, blank at the start. Don't use it otherwise, go directly to the persistent database.

Comment 17

[klonos]

I totally agree with the solution proposed in comment #16. The temporary in-memory db is the perfect thing to implement imo.

Comment 18

[Honza Bambas (:mayhemer)]

Attachment: v1

This is the first version of my proposal implementation. We now have 3 databases: 
1. one we had before, persistent sqlite
2. just in-memory used as an overlay of the persistent one when storage is used in session-only cookies mode, initially loaded with data from the persistent database
3. also just in-memory, used in private browsing mode exceptionally and wiped out on enter/exit of PB mode

The source code organization:
nsDOMStorageDB is now a wrapper that dispatches to one of the 3 databases by state and permissions for a storage. nsDOMStorageMemoryDB is in-memory implementation. nsDOMStoragePersistentDB is copy of the original nsDOMStorageDB, there are few small changes, I can provide a diff.

Patch includes new tests for this new stuff. Patch is based on patch from bug 488446.

Comment 19

[Alex Faaborg [:faaborg] (Firefox UX)]

So here is what I am worried about (although currently the problem is just theoretical)

1) user switches to private browsing mode
2) user creates a lot of content using an offline Web application (like a diagram, or a draft of a long email message)
3) user loses all of their data when the in-memory database is wiped on exist of private browsing mode

I get that this is sort of the point, and it's their own fault for creating content in a mode that destroys content, but nonetheless better user experiences would potentially:

-not allow the user to create data that will be destroyed in the first place
-not destroy data that the user creates regardless of mode (for instance we don't delete any bookmarks you create while you are in private browsing mode)

Comment 20

[(no longer active)]

(In reply to comment #19)
> -not allow the user to create data that will be destroyed in the first place

I think the concern here was that web apps might not be ready to handle exceptions in localStorage code, and actually throwing such exceptions may allow them to tell if the user is in private browsing mode.  Honza, please correct me if I'm wrong.

> -not destroy data that the user creates regardless of mode (for instance we
> don't delete any bookmarks you create while you are in private browsing mode)

The distinction between creating localStorage data and bookmarks is that you can create the latter without visiting any site, while creating the former implicates having visited the respective site.  So by storing the latter we're not violating our contract with users that we're not going to store any data which could be used to identify their browsing detail, whereas by storing the former we would.

Comment 21

[Honza Bambas (:mayhemer)]

Yes, one of the conditions for the solution is to disallow recognition of content that we are in session only cookies mode or private browsing mode. If we throw dom quota error it's in 99% cases indication we are in one of that modes, so I would like to find a different solution.

Maybe this: we let user decide about offline applications before it tries to store any data locally. This also, by a part, applies to dom storage. We raise quota for it when a domain is 'marked' as an offline application (user's decision). So, we may use this decision to allow store to localStorage/globalStorage even in PB mode (not in session only cookies mode). It's more or less under user's control. But, we may get to an inconsistent state here as we do not provide any cookies to an offline application that might be vital for it. Also, it can potentially be used to track users somehow when user allows this privilege for a violent web app.

Opinions?

Comment 22

[klonos]

What I see in this discussion is fear of loss-of-data, but this is only due to user error/forgetfulness. I mean after all it is the user that initiates the private mode. They shouldn't do this if they don't understand the risks. You are right though that we should do our best to inform the user of the risks, so what we do so far:

a) We have an informational pop-up telling the user that his current session will not be lost + the infrastructure to achieve that.

b) After entering private mode, we have a warning to inform the user of the risks. I think that the implementation of the same way the user is warned for an untrusted site is the best. So, we have a whole-page-warning that is displayed, not just a 'ok/cancel' pop-up (that all-average users are accustomed to click 'ok' to just to get quickly out off).

My only minor suggestion here is that this should actually behave the way the untrusted site warning does. It should also have a 'I understand/will be careful' link/button that when clinked takes the user to their home page. Not that important though since there is the home page button workaround.

A rather major suggestion here is the implementation of a yellow warning-ribbon below the tab bar, like the one displayed when a user tries to install an extension. While in private mode and in a situation/site where entered data might be lost due to forgetfulness (user understands what private mode is, but forgets he is in that mode), this ribbon with the warning should be displayed the whole time. If the user clicks on 'cancel/i understand the risks of private mode' (because he finds the warning ribbon annoying & space consuming), then its their mistake.

Almost all of the above are already implemented, so all that could be done to prevent bad things from happening to the user was done, we can still sleep at night IMHO and local storage should not be touched.

Comment 23

[Alex Faaborg [:faaborg] (Firefox UX)]

Creating an interface that doesn't allow the user to make a mistake is of course better than giving the user a lot of warning messages asking them not to make a mistake.

Is there anyway to communicate to the Web application that access to offline storage is not currently available, but not go as far to communicate that the user is in private browsing mode? (I'm assuming the answer is no, which is why we are in this quandary).

Comment 24

[(no longer active)]

(In reply to comment #23)
> Creating an interface that doesn't allow the user to make a mistake is of
> course better than giving the user a lot of warning messages asking them not to
> make a mistake.

Agreed.

> Is there anyway to communicate to the Web application that access to offline
> storage is not currently available,

Yes, there is: throwing on access to the localStorage object.

> but not go as far to communicate that the
> user is in private browsing mode?

This is the problem.  Honza, is there any other reason why a localStorage method would throw (a security error for example)?  If yes, how rare does that case happen?

Comment 25

[Honza Bambas (:mayhemer)]

(In reply to comment #24)
> (In reply to comment #23)
> > Is there anyway to communicate to the Web application that access to offline
> > storage is not currently available,
> 
> Yes, there is: throwing on access to the localStorage object.
> 

Bug 488446 does that, but it reveals your current settings (PB mode or session-only cookies mode) to content. But that's one of goals, not to allow that.

> > but not go as far to communicate that the
> > user is in private browsing mode?
> 
> This is the problem.  Honza, is there any other reason why a localStorage
> method would throw (a security error for example)?  If yes, how rare does that
> case happen?

As proposed many times and according to the spec:

"The setItem(key, value)" ... "If it couldn't set the new value, the method must raise an QUOTA_EXCEEDED_ERR exception. (Setting could fail if, e.g., the user has disabled storage for the domain, or if the quota has been exceeded.)"

This opens few issues:
- it is very likely that QUOTA_EXCEEDED_ERR indicates PB or SO mode, content may recognize it, for example when there is no data for an origin
- it is likely this will break some not well written offline apps, but it's not an argument
- when we provide existing data (in SO cookies mode), it is not clear what removeItem() and clear() methods should do; throw some kind of an error? probably not; silently fail and update the data? then it should be possible to store to the storage when we deleted from it as there is more space now, shouldn't it?

We could completely remove localStorage from DOM in SO/PB mode (not a simple task) but it is obvious for content to recognize we do it - user agent version indicates it supports localStorage but it's not available? No, we are back at the start.

Clearly, we have to behave like we would provide a clean profile that is deleted after PB mode is left. It's user's decision to enter it and leave it. The same applies to SO cookies mode, user doesn't wish a page/web app to store any data but let it work normally as it could.

No one replied to my comment 21 "Maybe this:", just for curiosity..

Comment 26

[Honza Bambas (:mayhemer)]

Comment on attachment 373658 [details] [diff] [review]
v1

We are getting close to code freeze and this patch is good enough to land. jst, will you find time to sr this please?

Comment 27

[Daniel Veditz [:dveditz]]

Comment on attachment 373658 [details] [diff] [review]
v1

I shouldn't count as a full r=, you need someone who knows this code (Neil?).

>+++ b/dom/src/storage/nsDOMStorageDB.h
>@@ -13,43 +13,46 @@
>  * Contributor(s):
>- *   Neil Deakin <[e-mail]>
>+ *   Honza Bambas <[e-mail]>

Why take out Neil's name here?

The direction represented here looks good to me, r+ on that.

Comment 28

[Honza Bambas (:mayhemer)]

(In reply to comment #27)
> (From update of attachment 373658 [details] [diff] [review])
> I shouldn't count as a full r=, you need someone who knows this code (Neil?).
> 

Thanks for the tip.

> >+++ b/dom/src/storage/nsDOMStorageDB.h
> >@@ -13,43 +13,46 @@
> >  * Contributor(s):
> >- *   Neil Deakin <[e-mail]>
> >+ *   Honza Bambas <[e-mail]>
> 
> Why take out Neil's name here?

Probably because it's a completely new file.

> 
> The direction represented here looks good to me, r+ on that.

Comment 29

[(no longer active)]

One question regarding the tests: why do you call nsIObserver::Observe directly with the "private-browsing" topic?  I really prefer that you use the private browsing service instead...

Comment 30

[Honza Bambas (:mayhemer)]

Comment on attachment 373658 [details] [diff] [review]
v1

Nail, will you find time to review this please?

Comment 31

[Honza Bambas (:mayhemer)]

(In reply to comment #29)
> One question regarding the tests: why do you call nsIObserver::Observe directly
> with the "private-browsing" topic?  I really prefer that you use the private
> browsing service instead...

I cannot do it, because it would switch the whole browser and I would loose the test environment w/o a way to return back to normal mode and continue test chain. I agree it's not perfect, but very well fits the needs.

Comment 32

[(no longer active)]

(In reply to comment #31)
> (In reply to comment #29)
> > One question regarding the tests: why do you call nsIObserver::Observe directly
> > with the "private-browsing" topic?  I really prefer that you use the private
> > browsing service instead...
> 
> I cannot do it, because it would switch the whole browser and I would loose the
> test environment w/o a way to return back to normal mode and continue test
> chain. I agree it's not perfect, but very well fits the needs.

You just need to set the browser.privatebrowsing.keep_current_session pref.  Please see <http://mxr.mozilla.org/mozilla-central/source/dom/tests/mochitest/bugs/iframe_bug463000.html?force=1#35> for a real life example.

Comment 33

[Honza Bambas (:mayhemer)]

Attachment: v1.1

- fixed the test, now using PB service to switch
- added one missing test
- it's no more based on patch from bug 488446 (now based on the trunk)

To note, the patch is 30% tests and 50% rename of nsDOMStorageDB to nsDOMStoragePersistentDB (the sql implementation) with few little cosmetic changes. Then there are two new classes, nsDOMStorageDB (a wrapper for PB, session only and sql DBs) and nsDOMStorageMemoryDB class (only in-memory DB).

Comment 34

[(no longer active)]

(In reply to comment #33)
> Created an attachment (id=377187) [details]
> v1.1
> 
> - fixed the test, now using PB service to switch

It seems that you had forgotten to include pbSwitch.js in your patch...

Comment 35

[Honza Bambas (:mayhemer)]

Attachment: v1.1 (added missing js test file)

Comment 36

[(no longer active)]

Sorry for the continuous nits from my part, but the way that pbSwitch.js works would cause these tests to fail in SeaMonkey and other apps which do not support the private browsing service yet.  You need to put the code inside get_PBSvc in a try/catch block and bail out of the test early one if the private browsing service is not available.

Comment 37

[Honza Bambas (:mayhemer)]

Ah, now I understand the try catch blocks. Ok, will do that. Ehsan, please feel free to take a deeper look into the patch and comment on it. I'll get to it back tomorrow anyway.

Comment 38

[Honza Bambas (:mayhemer)]

Attachment: v1.1 (more test fixes)

C code was not changed from the original v1.1.

Comment 39

[Johnny Stenback (:jst)]

Comment on attachment 377375 [details] [diff] [review]
v1.1 (more test fixes)

This patch looks good. Like Honza said, it's mostly a rename, with a new in-memory db, and some changes to use either persisten or in-memory storage. Code wise I think this looks very good over all. A couple of minor notes below.

Given that nsDOMStorageDB.{cpp,h} was effectively renamed nsDOMStoragePresistentDB.{cpp,h}, it'd probably be a good idea to do a hg mv on those before making the chages in the files to rename the class, that way history would be better preserved in hg. Please have a look at doing that, if it's a big pain, I wouldn't sweat it, but keeping history with the file would be nice if we can.
 
- In nsDOMStorageDB.h:

  * Contributor(s):
- *   Neil Deakin <[e-mail]>
+ *   Honza Bambas <[e-mail]>

If you do the remame in hg, and then add this as a new file, this change that looks really odd would go away :)

+nsDOMStorageMemoryDB::GetItemsTable(nsDOMStorage* aStorage,
+                              nsInMemoryStorage** aMemoryStorage)

Fix indentation of second line arguments. This is all over this file, if you have the cycles now, please do a pass over this file and fix the indentation.

+nsDOMStoragePersistentDB::RemoveKey(nsDOMStorage* aStorage,
+                          const nsAString& aKey,
+                          PRInt32 aKeyUsage)

Same thing, fix indentation if you've got the time to.

+ * Portions created by the Initial Developer are Copyright (C) 2006

All new files should have copyright year of 2009.

r+sr=jst with those minor issues looked into.

Comment 40

[Honza Bambas (:mayhemer)]

Attachment: v1.1 with proper rename [Checkin comment 42]

I wasn't able to rename nsDOMStorageDB to nsDOMStoragePeristentDB and then create a new nsDOMStorageDB file (with name of the original file). So, I did the renaming but new files are nsDOMStorageDBWrapper .cpp and .h, including rename of the class to nsDOMStorageDBWrapper. Please only confirm this change, the rest of the patch is intact, except your review comments.

Comment 41

[Johnny Stenback (:jst)]

Comment on attachment 377914 [details] [diff] [review]
v1.1 with proper rename [Checkin comment 42]

Looks good!

Comment 42

[Honza Bambas (:mayhemer)]

Comment on attachment 377914 [details] [diff] [review]
v1.1 with proper rename [Checkin comment 42]

http://hg.mozilla.org/mozilla-central/rev/e0251c29291c

Comment 43

[Honza Bambas (:mayhemer)]

Attachment: test fix [Checkin comment 43]

http://hg.mozilla.org/mozilla-central/rev/c5288fb4332e

Comment 44

[Honza Bambas (:mayhemer)]

Attachment: v1.1 + test fix for 1.9.1

Based on top of patch for bug 492219.

Comment 45

[Honza Bambas (:mayhemer)]

Attachment: v1.1 as landed on 1.9.1 [Checkin comment 45]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9f79ae84d90b

Comment 46

[Eric Shepherd [:sheppy]]

Just so I'm clear on the gist of this, let me check this:

Basically, the point of this from a documentation perspective is that in session-only cookies mode, you start with a copy of the regular database, but changes are thrown out when you leave session-only mode... and private browsing mode starts with an empty database, which is thrown away when you exit private browsing mode.

Correct?

Comment 47

[Honza Bambas (:mayhemer)]

(In reply to comment #46)
> Just so I'm clear on the gist of this, let me check this:
> 
> Basically, the point of this from a documentation perspective is that in
> session-only cookies mode, you start with a copy of the regular database, but
> changes are thrown out when you leave session-only mode... and private browsing
> mode starts with an empty database, which is thrown away when you exit private
> browsing mode.
> 
> Correct?

Absolutely correct.

Comment 48

[Eric Shepherd [:sheppy]]

OK, I've added appropriate notes in all of the following articles:

https://developer.mozilla.org/En/NsIPrivateBrowsingService
https://developer.mozilla.org/en/Supporting_private_browsing_mode
https://developer.mozilla.org/en/DOM/Storage#localStorage

Comment 49

[Honza Bambas (:mayhemer)]

(In reply to comment #48)
> OK, I've added appropriate notes in all of the following articles:
> 
> https://developer.mozilla.org/En/NsIPrivateBrowsingService

Sentence "while the temporary local storage database starts as a copy of the regular local storage database" is wrong, localStorage start also empty (as cookies) in PB mode.

> https://developer.mozilla.org/en/Supporting_private_browsing_mode

The same here.

> https://developer.mozilla.org/en/DOM/Storage#localStorage

Article "Note: When the browser goes into private browsing mode, a new, temporary database is created to store local storage data. The database starts as a copy of the regular local storage database, but is thrown away when private browsing mode is turned off." as well wrong.

Only in session-only-cookies mode we start with a copy of existing data that is thrown away when leaving that mode.

Comment 50

[Eric Shepherd [:sheppy]]

I've fixed those issues. Thank you.