Closed Bug 910088 Opened 7 years ago Closed 7 years ago

Use webIDL dictionaries for DOMWindowResize event details

Categories

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

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26
Tracking Status
firefox25 --- unaffected
firefox26 + fixed

People

(Reporter: kanru, Assigned: kanru)

References

Details

Attachments

(1 file, 2 obsolete files)

This needs to happen.  The situation as done in bug 891763 is not OK, and we shouldn't be shipping it.
Assignee: nobody → kchen
More precisely, I don't think the patch in bug 891763 should have gotten r+.
Do we want to backout the landing in bug 891763? It only landed to trunk 5 hours ago.
Flags: needinfo?(bzbarsky)
No, I don't think things are that dire.
Flags: needinfo?(bzbarsky)
(In reply to Boris Zbarsky [:bz] from comment #2)
> More precisely, I don't think the patch in bug 891763 should have gotten r+.

I'm really sorry about that.

While we're here we should probably also fix nsIAsyncScrollEventDetail.
What's the problem with nsIAsyncScrollEventDetail?
I mean other than it not being used afaict.
(In reply to Boris Zbarsky [:bz] from comment #6)
> What's the problem with nsIAsyncScrollEventDetail?

Oh, I see that it's been converted and moved into BrowserElementDictionaries.webidl.

> I mean other than it not being used afaict.

So I guess nothing, aside from this.  :)
Attached patch Patch (obsolete) — Splinter Review
Attachment #797041 - Flags: review?(bzbarsky)
Attached patch Patch (obsolete) — Splinter Review
Use new API
Attachment #797041 - Attachment is obsolete: true
Attachment #797041 - Flags: review?(bzbarsky)
Attachment #797043 - Flags: review?(bzbarsky)
Comment on attachment 797043 [details] [diff] [review]
Patch

>+  AutoSafeJSContext cx;

I think you need to enter the compartment of the window's JSObject on this cx.

>+  detail.ToObject(cx, JS::NullPtr(), &detailValue);

And if this returns false, return false.

r=me with those fixed.  Thank you!
Attachment #797043 - Flags: review?(bzbarsky) → review+
Attached patch Patch to commitSplinter Review
Attachment #797043 - Attachment is obsolete: true
Attachment #797083 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/f3d8e568ec11
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
(In reply to Boris Zbarsky [:bz] from comment #11)
> >+  detail.ToObject(cx, JS::NullPtr(), &detailValue);
> 
> And if this returns false, return false.

Why did you not address this comment?
Flags: needinfo?(kchen)
(In reply to :Ms2ger from comment #15)
> (In reply to Boris Zbarsky [:bz] from comment #11)
> > >+  detail.ToObject(cx, JS::NullPtr(), &detailValue);
> > 
> > And if this returns false, return false.
> 
> Why did you not address this comment?

Oops. I forgot it. Sorry.
Flags: needinfo?(kchen)
https://hg.mozilla.org/integration/b2g-inbound/rev/08227e64ea54
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/08227e64ea54
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.