Closed Bug 1225188 Opened 4 years ago Closed 4 years ago

Add window.onstorage

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

We fire storage events and have for a long time.  But there is no window.onstorage.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #8688012 - Flags: review?(bugs) → review+
I am getting one web platform test failure with this patch:

 TEST-UNEXPECTED-FAIL | /webstorage/event_body_attribute.html | sessionStorage mutations fire StorageEvents that are caught by the event listener specified as an attribute on the body. - assert_unreached: got at least 2, expected only 1 events Reached unreachable code 

but as far as I can tell this test is just buggy.  It does the following:

1)  iframe = document.createElement("IFRAME");
    iframe.src = "about:blank";
    document.body.appendChild(iframe);
    storageEventList = new Array();
    iframe.contentWindow.addEventListener("storage", function(e) {
       window.parent.storageEventList.push(e);
    });

2)  Synchronously (before the above load of about:blank completes) loads a document in the iframe that contains:

   <script>
     function handleStorageEvent(e) {
        window.parent.storageEventList.push(e);
     }
   </script>
   <body onstorage="handleStorageEvent(window.event);">

Note that at this point, since this document reuses the same inner that was already loaded in the subframe, there are _two_ listeners for the "storage" event on that window.

3) Changes sessionStorage, then asserts that storageEventList.length is 1.  This assertion fails since both event listeners above added things to the list.

I looked at the blame for this code and it looks like the test used to set onstorage both places (so one of the sets was a no-op), but then ms2ger changed it in rev 2fc1a87d8eef6d55e8daac3d1a0a32f8a6b735d1 of wpt.  So then the test could be passed only by an implementation that didn't actually support the onstorage handler.

The test also has two other problems:

1)  The chunk in eventTestHarness.js can't just be removed because that file is used by other tests in this directory.

2)  The actual event handler in event_body_handler.html uses window.event.

So I would propose the following:

A) Revert wpt rev 2fc1a87d8eef6d55e8daac3d1a0a32f8a6b735d1.
B) Change this test to null out iframe.contentWindow.onstorage before doing the load in the subframe.
C) Change the subframe to not use window.event.

Thoughts?
Flags: needinfo?(Ms2ger)
I'm thinking of rewriting those tests at some point, but the change as landed looks good to me.
Flags: needinfo?(Ms2ger)
https://hg.mozilla.org/mozilla-central/rev/96fb5c404dfb
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
I've documented this here:

https://developer.mozilla.org/en-US/docs/Web/API/Window/onstorage

And added a note to the release notes:

https://developer.mozilla.org/en-US/Firefox/Releases/45#Miscellaneous

Question - is this specced in a standard anywhere; is it proprietary?
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.