Last Comment Bug 675884 - Implement Event constructors
: Implement Event constructors
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Olli Pettay [:smaug]
:
Mentors:
Depends on:
Blocks: 708701
  Show dependency treegraph
 
Reported: 2011-08-02 02:06 PDT by Anne (:annevk)
Modified: 2015-01-03 02:40 PST (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (17.60 KB, patch)
2011-12-08 09:37 PST, Olli Pettay [:smaug]
no flags Details | Diff | Review
patch (18.28 KB, patch)
2011-12-08 10:07 PST, Olli Pettay [:smaug]
jst: review+
Details | Diff | Review
using nsDependentJSString (1.67 KB, patch)
2011-12-11 05:20 PST, Olli Pettay [:smaug]
jst: review+
Details | Diff | Review

Description Anne (:annevk) 2011-08-02 02:06:43 PDT
DOM Core introduces a constructor syntax for Events.

http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#dom-event

Please implement it so init*Event() can die in a fire.
Comment 1 Olli Pettay [:smaug] 2011-12-08 08:26:39 PST
Patch coming soon for Event and CustomEvent.
I don't know specs defining other event ctors.
Comment 2 Olli Pettay [:smaug] 2011-12-08 08:34:09 PST
Ah, there are other specs for other event ctors.

But I'll start from Event and CustomEvent.
Comment 3 Olli Pettay [:smaug] 2011-12-08 09:37:31 PST
Created attachment 580075 [details] [diff] [review]
patch

This adds the basic stuff (Event and CustomEvent and ctors are defined in DOM4).
After this adding support for new event ctors is easy:
- Create FooEventInit interface
- Add NS_DEFINE_EVENT_CTOR(Foo) and NS_DEFINE_EVENT_CONSTRUCTOR_FUNC_DATA(Foo)
  to nsDOMClassInfo.cpp
- virtual const nsIID& EventInitIID() { return NS_GET_IID(nsIFooEventInit); } 
  to the nsDOMFooEvent.h
- Implement
  virtual nsresult InitFromCtor(const nsAString& aType, nsISupports* aDict);
Comment 4 Olli Pettay [:smaug] 2011-12-08 09:44:00 PST
https://tbpl.mozilla.org/?tree=Try&rev=109e5fb30913
Comment 5 :Ms2ger 2011-12-08 09:53:37 PST
Can you check the expected behaviour for { bubbles: get () { throw foo } } and add a test?

Also, \o/
Comment 6 Olli Pettay [:smaug] 2011-12-08 10:07:20 PST
Created attachment 580086 [details] [diff] [review]
patch

Handle exceptions per WebIDL
Comment 7 :Ms2ger 2011-12-08 10:23:20 PST
Comment on attachment 580086 [details] [diff] [review]
patch

>--- a/content/events/src/nsDOMEvent.cpp
>+++ b/content/events/src/nsDOMEvent.cpp
>+nsDOMEvent::Initialize(nsISupports* aOwner, JSContext* aCx, JSObject* aObj,
>+                       PRUint32 aArgc, jsval* aArgv)
>+{
>+  JSAutoRequest ar(aCx);
>+  JSString* jsstr = JS_ValueToString(aCx, aArgv[0]);
>+  if (!jsstr) {
>+    return NS_ERROR_DOM_SYNTAX_ERR;
>+  }
>+
>+  JS::Anchor<JSString*> deleteProtector(jsstr);
>+  size_t length;
>+  const jschar* chars = JS_GetStringCharsAndLength(aCx, jsstr, &length);
>+  if (!chars) {
>+    return NS_ERROR_OUT_OF_MEMORY;
>+  }
>+
>+  nsAutoString type;
>+  type.Assign(chars, length);
>+  deleteProtector.clear();

Can this use nsDependentJSString?
Comment 8 Olli Pettay [:smaug] 2011-12-08 10:40:43 PST
I don't know why nsDependentJSString is ever safe to use.
Comment 9 Olli Pettay [:smaug] 2011-12-09 14:26:03 PST
I'm trying to write a code generator for event ctors.

But feel free to review the current patches.
Comment 10 Olli Pettay [:smaug] 2011-12-11 04:48:48 PST
I filed Bug 709569 to get a tool for generic dictionary handling.
Comment 11 Olli Pettay [:smaug] 2011-12-11 05:20:48 PST
Created attachment 580731 [details] [diff] [review]
using nsDependentJSString

I'll file a followup bug to get better nsDependentJSString <-> JS::Anchor 
handling. nsDependentJSString could just do the right thing with JSString, or at 
least  enforce that JS::Anchor is used.
Comment 12 Olli Pettay [:smaug] 2011-12-11 09:10:35 PST
Comment on attachment 580086 [details] [diff] [review]
patch

er, the previous patch is on top of this.
Comment 14 :Ms2ger 2013-03-30 06:45:53 PDT
We really need serious documentation for this...

Note You need to log in before you can comment on or make changes to this bug.