Closed
Bug 761729
Opened 13 years ago
Closed 13 years ago
Either implement mozIDOMApplicationEvent in C++, or make WebApps use CustomEvent
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: smaug, Assigned: fabrice)
References
Details
Attachments
(1 file, 2 obsolete files)
13.31 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.js#399
DOMEvents will be forced to be C++ only. Right now properly implemented DOM Events
are already C++ only, but apparently there is at least one JS implementation too
(which doesn't work at all with event dispatching code).
Updated•13 years ago
|
Component: Web Apps → DOM: Mozilla Extensions
Product: Firefox → Core
QA Contact: webapps → general
Assignee | ||
Comment 1•13 years ago
|
||
I had to refactor the IDL to move the Application object definition in its own file since it is use in the dictionnary, hence the include change to nsGlobalWindow.h
Assignee: nobody → fabrice
Attachment #633215 -
Flags: review?(bugs)
Reporter | ||
Comment 2•13 years ago
|
||
Comment on attachment 633215 [details] [diff] [review]
patch
>+[scriptable, uuid(b70b84f1-7ac9-4a92-bc32-8b6a7eb7879e)]
>+interface mozIDOMApplication : nsISupports
Nit, extra space before :
>+{
>+ readonly attribute jsval manifest;
>+ readonly attribute DOMString manifestURL;
>+ readonly attribute nsIArray receipts; /* an array of strings */
Not about this bug, but huh. Why do we expose nsIArray to web. We really shouldn't do that.
But I don't understand why you need to move Application interface.
Assignee | ||
Comment 3•13 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #2)
> Not about this bug, but huh. Why do we expose nsIArray to web. We really
> shouldn't do that.
Yep, I plan to move that to a jsval.
>
> But I don't understand why you need to move Application interface.
since the dictionnary is :
{
mozIDOMApplication application;
}
and mozIDOMApplication is not a built-in type, the code generator that uses dictionary_helper_gen.conf looks for a "mozIDOMApplication.h" header file.
Reporter | ||
Comment 4•13 years ago
|
||
Couldn't you add mozIDOMApplication to exclude_automatic_type_include
and perhaps nsIDOMApplicationRegistry.h to special_includes
Reporter | ||
Comment 5•13 years ago
|
||
...in dictionary_helper_gen.conf
Assignee | ||
Comment 6•13 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #5)
> ...in dictionary_helper_gen.conf
I didn't know about these. I'll update my patch, thanks!
Assignee | ||
Comment 7•13 years ago
|
||
Updated to use exclude_automatic_type_include.
Attachment #633215 -
Attachment is obsolete: true
Attachment #633215 -
Flags: review?(bugs)
Attachment #633285 -
Flags: review?(bugs)
Reporter | ||
Comment 8•13 years ago
|
||
Comment on attachment 633285 [details] [diff] [review]
patch v2
>+nsDOMMozApplicationEvent::GetApplication(mozIDOMApplication **aApplication)
Nit, mozIDOMApplication **aApplication
>+private:
>+ nsCOMPtr<mozIDOMApplication> mApplication;
Almost missed... you have strong pointer to Application, so you need to add it to
cycle collection. (You can check nsDOMCustomEvent as an example. It has mDetail as a strong member variable.)
Attachment #633285 -
Flags: review?(bugs) → review-
Assignee | ||
Comment 9•13 years ago
|
||
Now with cycle collection for mApplication.
Attachment #633285 -
Attachment is obsolete: true
Attachment #633297 -
Flags: review?(bugs)
Reporter | ||
Updated•13 years ago
|
Attachment #633297 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 10•13 years ago
|
||
Comment 11•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
Updated•12 years ago
|
Component: DOM: Mozilla Extensions → DOM
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•