RESOLVED FIXED 211503
Regression(r254859) DOM storage event gets fired at the frame that caused the storage modification
https://bugs.webkit.org/show_bug.cgi?id=211503
Summary Regression(r254859) DOM storage event gets fired at the frame that caused the...
pirimoglu
Reported 2020-05-06 03:47:43 PDT
With Safari 13.1 update, DOM storage event is being raised at the same window that has caused the storage event to happen. This is a behavior difference from earlier versions of Safari, and is not complaint with the HTML Living Standard specification on this. The standard spec at https://html.spec.whatwg.org/multipage/webstorage.html#localStorageEvent says this for localStorage events: "When the setItem(), removeItem(), and clear() methods are called on a Storage object x that is associated with a local storage area, if the methods did not throw an exception or "do nothing" as defined above, then for every Document object whose relevant global object's localStorage attribute's Storage object is associated with the same storage area, other than x, send a storage notification." And says this for sessionStorage events: "When the setItem(), removeItem(), and clear() methods are called on a Storage object x that is associated with a session storage area, if the methods did not throw an exception or "do nothing" as defined above, then for every Document object whose relevant global object's sessionStorage attribute's Storage object is associated with the same storage area, other than x, send a storage notification." For both localStorage and sessionStorage, the a "storage notification" should be sent for every document whose corresponding storage object is the same one as the current document, which they call "x", but not on "x". So the document who made the setItem(), removeItem() or clear() methods, should not receive the storage notification (event). Only others that are in the same session who share the same storage object should receive the notification. Before Safari 13.1 it was behaving this way. With Safari 13.1 it is not behaving this way, the document that called the methods that causes the notification (event) is also receiving the event. To give example in js code: Assume document A has this code: window.addEventListener("storage", storageEventHandler); function storageEventHandler(e) { console.log("A received " + e.key); } function AddToLocalStorage(key, value) { localStorage.setItem(key, value); } And document B has this code, which is loaded from same origin/site as document A, so they share session and so localStorage: window.addEventListener("storage", storageEventHandler); function storageEventHandler(e) { console.log("B received " + e.key); } Assume both document A and document B are loaded and the event listeners are set. Assume on document A, AddToLocalStorage("new key" , "value1") is called, by a button click or some other way. In this case, because document A is calling setItem(), document A's storegaEventHandler() should not be invoked. But document B's storageEventHandler() should be invoked. With Safari 13.1 document A's storageEventHandler is being invoked too. This is against the specification, and breaks existing functionality that relies on this. Only IE 10 behaves this way, but it is known to not be complaint with specifications.
Attachments
Patch (8.73 KB, patch)
2020-05-21 16:37 PDT, Chris Dumez
no flags
Radar WebKit Bug Importer
Comment 1 2020-05-07 10:53:32 PDT
Chris Dumez
Comment 2 2020-05-21 15:06:01 PDT
Will fix shortly. Thanks for the bug report.
Chris Dumez
Comment 3 2020-05-21 16:37:30 PDT
Chris Dumez
Comment 4 2020-05-21 16:51:53 PDT
pirimoglu@hotmail.com: I don't think we shipped this bug yet. Were you testing Safari Technology Preview?
pirimoglu
Comment 5 2020-05-22 00:18:06 PDT
No, released product. It starts with Safari 13.1, from end user complaints and what we tested.
pirimoglu
Comment 6 2020-05-22 01:45:10 PDT
I think you are testing your fix with only window.sessionStorage, but not with window.localStorage. I went back and tested this with the Safari 13.1, both with sessionStorage and localStorage. In Safari 13.1, sessionStorage is working as expected. But localStorage has this problem, which is what our javascript is using on our site.
Maciej Stachowiak
Comment 7 2020-05-22 01:46:45 PDT
Comment on attachment 399997 [details] Patch r=me
EWS
Comment 8 2020-05-22 07:28:40 PDT
commit-queue failed to commit attachment 399997 [details] to WebKit repository.
Chris Dumez
Comment 9 2020-05-22 08:17:15 PDT
Comment on attachment 399997 [details] Patch Clearing flags on attachment: 399997 Committed r262058: <https://trac.webkit.org/changeset/262058>
Chris Dumez
Comment 10 2020-05-22 08:17:17 PDT
All reviewed patches have been landed. Closing bug.
pirimoglu
Comment 11 2020-05-22 09:02:20 PDT
Can you please confirm or verify if the fix with localStorage still work when there are two or more browser tabs open that share the same localStorage, due to being on same url or same origin? If one of these windows had modified the localStorage, the other windows should receive the storage event, but not the originating window. Our javascript code is relying on this. Wanted to confirm this will still be case and not regress. Just in case this may be behaving differently than iframe on the same page case. Thanks for the fix!
Chris Dumez
Comment 12 2020-05-22 11:54:53 PDT
(In reply to pirimoglu from comment #11) > Can you please confirm or verify if the fix with localStorage still work > when there are two or more browser tabs open that share the same > localStorage, due to being on same url or same origin? > > If one of these windows had modified the localStorage, the other windows > should receive the storage event, but not the originating window. Our > javascript code is relying on this. Wanted to confirm this will still be > case and not regress. > > Just in case this may be behaving differently than iframe on the same page > case. > > Thanks for the fix! Yes, I have just verified that it works as expected, even with multiple tabs. You should be able to validate this too in the next Safari Technology Preview build.
pirimoglu
Comment 13 2020-05-22 11:59:06 PDT
Thanks again!
Chris Dumez
Comment 14 2020-11-23 10:02:26 PST
*** Bug 210512 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.