User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0 Build ID: 20160923210021 Steps to reproduce: 1. Add a "dom-storage2-changed" observer 2. Start a normal browsing window 3. Start a private browsing window 4. Browse Actual results: "dom-storage2-changed" notifications are received from both normal and private windows Expected results: There should be two different notifications: "dom-storage2-changed" and "private-dom-storage2-changed" like it was done for cookies in bug 837091 Or notifications should indicate whether they relate to normal or private window Or notifications should only be received from normal windows

(In reply to JustOff from comment #0) > User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 > Firefox/48.0 > Build ID: 20160923210021 > > Steps to reproduce: > > 1. Add a "dom-storage2-changed" observer > 2. Start a normal browsing window > 3. Start a private browsing window > 4. Browse > > > Actual results: > > "dom-storage2-changed" notifications are received from both normal and > private windows > > > Expected results: > > There should be two different notifications: "dom-storage2-changed" and > "private-dom-storage2-changed" like it was done for cookies in bug 837091 > > Or notifications should indicate whether they relate to normal or private > window > > Or notifications should only be received from normal windows Hi :baku, what do you think about the expected behaviours? Any idea of what we want to do here? Thanks!

Also private windows have localStorage and sessionStorage. I don't why they should receive a different notification. dom-storage2-changed is an internal notification. having 2 different notification, is it useful for some other components?

@Andrea, it's useful for addons such as https://addons.mozilla.org/addon/self-destructing-cookies/ and https://addons.mozilla.org/addon/cookies-exterminator/ and so was done for cookies in bug 837091, is this not enough reason?

The use case is that if you are running in a private window and receive this notification relating to a change in a non-private window, then you are going to be very confused because the change will not be visible to you. The local storage areas in private and non-private windows are completely separate and have different contents. Of course dom-storage2-changed may also be received in response to session storage changes that are not visible in a particular window, so overall its usefulness is limited. The MozSessionStorageChanged and MozLocalStorageChanged events (or MozStorageChanged before FF44) are much easier to work with at an individual window level. Or even the somewhat bizarrely-specified but official StorageEvent, which is only received in windows *other than* the one the storage area is in.

(In reply to JustOff from comment #3) > @Andrea, it's useful for addons such as > https://addons.mozilla.org/addon/self-destructing-cookies/ and > https://addons.mozilla.org/addon/cookies-exterminator/ and so was done for > cookies in bug 837091, is this not enough reason? Is the self-destructing-cookies affected by this, or is a potential issue?

NI for comment 5

I'm not an author of Self-Destructing Cookies, but yes, SDC is listening for dom-storage2-changed (https://addons.mozilla.org/en-US/firefox/files/browse/524654/file/lib/main.js#L247) to track and then to remove localStorage objects. Since SDC can't distinguish the private objects from the normal, it tries to do that for all recorded objects, regardless of their existance (https://addons.mozilla.org/en-US/firefox/files/browse/524654/file/lib/domstorage.js#L117, https://addons.mozilla.org/en-US/firefox/files/browse/524654/file/lib/domstorage.js#L302). It's not effective workaround, it wastes cpu and memory, even though that works.

In this case, I guess, we can have 2 separate notifications.

Janv, any news?

Comment on attachment 8808529 [details] [diff] [review] storage.patch Review of attachment 8808529 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/storage/DOMStorage.cpp @@ +12,5 @@ > #include "nsIScriptSecurityManager.h" > #include "nsIPermissionManager.h" > #include "nsIPrincipal.h" > #include "nsICookiePermission.h" > +#include "nsGlobalWindow.h" Nit: since this is not an interface it should technically go to the next include section. @@ +188,5 @@ > > private: > nsCOMPtr<nsISupports> mSubject; > const char16_t* mType; > + bool mPrivateBrowsing; Nit: const bool @@ +228,5 @@ > RefPtr<StorageEvent> event = > StorageEvent::Constructor(nullptr, NS_LITERAL_STRING("storage"), dict); > > + bool privateBrowsing = > + mWindow && nsGlobalWindow::Cast(mWindow)->IsPrivateBrowsing(); I'm a bit nervous about this |if mWindow &&| What if mWindow is null ? We will treat it as normal storage ? There's DOMStorage::IsPrivate() method which can be used instead. If you change it then you don't need to include nsGlobalWindow.h

Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/a7623a66ba08 dom-private-storage2-changed notification, r=janv