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)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: smaug, Assigned: fabrice)

References

Details

Attachments

(1 file, 2 obsolete files)

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).
Component: Web Apps → DOM: Mozilla Extensions
Product: Firefox → Core
QA Contact: webapps → general
Attached patch patch (obsolete) — Splinter Review
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)
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.
(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.
Couldn't you add mozIDOMApplication to exclude_automatic_type_include and perhaps nsIDOMApplicationRegistry.h to special_includes
...in dictionary_helper_gen.conf
(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!
Attached patch patch v2 (obsolete) — Splinter Review
Updated to use exclude_automatic_type_include.
Attachment #633215 - Attachment is obsolete: true
Attachment #633215 - Flags: review?(bugs)
Attachment #633285 - Flags: review?(bugs)
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-
Attached patch patch v3Splinter Review
Now with cycle collection for mApplication.
Attachment #633285 - Attachment is obsolete: true
Attachment #633297 - Flags: review?(bugs)
Attachment #633297 - Flags: review?(bugs) → review+
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
Component: DOM: Mozilla Extensions → DOM
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: