Closed
Bug 1014657
Opened 10 years ago
Closed 10 years ago
Port DOMStorageEvent to WebIDL and remove nsIDOMStorageEvent
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: baku, Assigned: baku)
References
Details
Attachments
(1 file, 1 obsolete file)
19.28 KB,
patch
|
Details | Diff | Splinter Review |
This is useful for porting DOM Storage to WebIDL.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8427101 -
Flags: review?(Ms2ger)
Assignee | ||
Updated•10 years ago
|
Comment 2•10 years ago
|
||
Comment on attachment 8427101 [details] [diff] [review] storageEvent.patch Review of attachment 8427101 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsGlobalWindow.cpp @@ +11321,5 @@ > } > > nsresult > nsGlobalWindow::CloneStorageEvent(const nsAString& aType, > + nsRefPtr<StorageEvent>& aEvent) Weird API... I think you could take StorageEvent* and return already_AddRefed<StorageEvent>, and that'd be clearer @@ +11330,3 @@ > > nsCOMPtr<nsIDOMEvent> domEvent = do_QueryInterface(aEvent, &rv); > NS_ENSURE_SUCCESS(rv, rv); I don't think you need this @@ +11330,5 @@ > > nsCOMPtr<nsIDOMEvent> domEvent = do_QueryInterface(aEvent, &rv); > NS_ENSURE_SUCCESS(rv, rv); > > + domEvent->GetBubbles(&dict.mBubbles); Bubbles(), Cancelable() @@ +11339,5 @@ > + aEvent->GetNewValue(dict.mNewValue); > + aEvent->GetUrl(dict.mUrl); > + dict.mStorageArea = aEvent->GetStorageArea(); > + > + aEvent = StorageEvent::Constructor(this, aType, dict); Can't this throw? ::: dom/events/EventDispatcher.cpp @@ -831,5 @@ > if (aEventType.LowerCaseEqualsLiteral("mozsmsevent")) > return NS_NewDOMMozSmsEvent(aDOMEvent, aOwner, aPresContext, nullptr); > if (aEventType.LowerCaseEqualsLiteral("mozmmsevent")) > return NS_NewDOMMozMmsEvent(aDOMEvent, aOwner, aPresContext, nullptr); > - if (aEventType.LowerCaseEqualsLiteral("storageevent")) { Need to ask smaug about this
Attachment #8427101 -
Flags: review?(bugs)
Attachment #8427101 -
Flags: review?(Ms2ger)
Attachment #8427101 -
Flags: feedback+
Comment 3•10 years ago
|
||
So this will break at least https://mxr.mozilla.org/addons/source/162068/chrome/dataman/content/dataman/dataman.js#2602 And I wouldn't be surprised if some binary addons use the .idl interface too.
Comment 4•10 years ago
|
||
Comment on attachment 8427101 [details] [diff] [review] storageEvent.patch But I think this should be ok. Binary addons just need to use JS for this stuff if needed. blink doesn't seem to support createEvent("storageevent") either, so removing that is hopefully ok. This is risky, but not sure what we could do to make it less risky.
Attachment #8427101 -
Flags: review?(bugs) → review+
Comment 5•10 years ago
|
||
(and please use Bubbles(), Cancelable() )
Assignee | ||
Comment 6•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=d909a5893202
Attachment #8427101 -
Attachment is obsolete: true
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2b9e7e71113
Comment 8•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f2b9e7e71113
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Comment 9•10 years ago
|
||
Could we back this out and have a patch which supports createEvent. Apparently it is needed after all - at least for now. Perhaps worth to re-check whether other engines have init*Event.
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•