Closed
Bug 975688
Opened 10 years ago
Closed 10 years ago
Move all nsDOM*Event classes into mozilla::dom namespace and rename them to same as standard spec's name (i.e., nsDOM*Event -> mozilla::dom::*Event)
Categories
(Core :: DOM: Events, enhancement)
Core
DOM: Events
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: masayuki, Assigned: masayuki)
References
Details
Attachments
(28 files)
If you don't disagree with the summary, please comment here as soon as possible.
Comment 1•10 years ago
|
||
I don't disagree.
Assignee | ||
Comment 2•10 years ago
|
||
Oops, I meant that you don't agree with the summary :-(
Assignee | ||
Updated•10 years ago
|
Whiteboard: [leave open]
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8381358 -
Flags: review?(bugs)
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #8381359 -
Flags: review?(bugs)
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8381360 -
Flags: review?(bugs)
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8381361 -
Flags: review?(bugs)
Assignee | ||
Comment 7•10 years ago
|
||
Attachment #8381362 -
Flags: review?(bugs)
Assignee | ||
Comment 8•10 years ago
|
||
Attachment #8381363 -
Flags: review?(bugs)
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8381364 -
Flags: review?(bugs)
Assignee | ||
Comment 10•10 years ago
|
||
It might be better to move NS_NewDOM*Event into mozilla::dom, but anyway, we should focus to renaming nsDOM*Event for now.
Comment 11•10 years ago
|
||
Comment on attachment 8381358 [details] [diff] [review] part.1 Rename DOMWheelEvent to WheelEvent > 'WheelEvent': { >- 'headerFile': 'DOMWheelEvent.h', >- 'nativeType': 'mozilla::dom::DOMWheelEvent', >+ 'headerFile': 'WheelEvent.h', >+ 'nativeType': 'mozilla::dom::WheelEvent', > }, I don't think we need this entry in Bindings.conf anymore, since headerFile name and nativeType are now the defaults.
Attachment #8381358 -
Flags: review?(bugs) → review+
Comment 12•10 years ago
|
||
Or, I guess you need to export WheelEvent.h to mozilla/dom so that bindings can use mozilla/dom/WheelEvent.h
Comment 13•10 years ago
|
||
Comment on attachment 8381359 [details] [diff] [review] part.2 Rename nsDOMAnimationEvent to mozilla::dom::AnimationEvent Same with this, and I assume with others. You could upload a new patch to remove stuff from Bindings.conf and make moz.build to export headers using EXPORTS.mozilla.dom
Attachment #8381359 -
Flags: review?(bugs) → review+
Updated•10 years ago
|
Attachment #8381360 -
Flags: review?(bugs) → review+
Updated•10 years ago
|
Attachment #8381361 -
Flags: review?(bugs) → review+
Updated•10 years ago
|
Attachment #8381362 -
Flags: review?(bugs) → review+
Updated•10 years ago
|
Attachment #8381363 -
Flags: review?(bugs) → review+
Updated•10 years ago
|
Attachment #8381364 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 14•10 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #12) > Or, I guess you need to export WheelEvent.h to mozilla/dom so that > bindings can use mozilla/dom/WheelEvent.h (In reply to Olli Pettay [:smaug] from comment #13) > Comment on attachment 8381359 [details] [diff] [review] > part.2 Rename nsDOMAnimationEvent to mozilla::dom::AnimationEvent > > Same with this, and I assume with others. > You could upload a new patch to remove stuff from Bindings.conf > and make moz.build to export headers using EXPORTS.mozilla.dom Yes, I like this idea better too!
Assignee | ||
Comment 15•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d047b89102aa https://hg.mozilla.org/integration/mozilla-inbound/rev/8fb9f057658a https://hg.mozilla.org/integration/mozilla-inbound/rev/4966af832a78 https://hg.mozilla.org/integration/mozilla-inbound/rev/6abc19b10e91 https://hg.mozilla.org/integration/mozilla-inbound/rev/7af67d41d918 https://hg.mozilla.org/integration/mozilla-inbound/rev/15320fc9ba1c https://hg.mozilla.org/integration/mozilla-inbound/rev/e41a9f01d217
Comment 16•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d047b89102aa https://hg.mozilla.org/mozilla-central/rev/8fb9f057658a https://hg.mozilla.org/mozilla-central/rev/4966af832a78 https://hg.mozilla.org/mozilla-central/rev/6abc19b10e91 https://hg.mozilla.org/mozilla-central/rev/7af67d41d918 https://hg.mozilla.org/mozilla-central/rev/15320fc9ba1c https://hg.mozilla.org/mozilla-central/rev/e41a9f01d217
Assignee | ||
Comment 17•10 years ago
|
||
Assignee | ||
Comment 18•10 years ago
|
||
Assignee | ||
Comment 19•10 years ago
|
||
Assignee | ||
Comment 20•10 years ago
|
||
Assignee | ||
Comment 21•10 years ago
|
||
Assignee | ||
Comment 22•10 years ago
|
||
Assignee | ||
Comment 23•10 years ago
|
||
Assignee | ||
Comment 24•10 years ago
|
||
Assignee | ||
Comment 25•10 years ago
|
||
Assignee | ||
Comment 26•10 years ago
|
||
Assignee | ||
Comment 27•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8382202 -
Flags: review?(bugs)
Assignee | ||
Updated•10 years ago
|
Attachment #8382205 -
Flags: review?(bugs)
Assignee | ||
Updated•10 years ago
|
Attachment #8382206 -
Flags: review?(bugs)
Assignee | ||
Updated•10 years ago
|
Attachment #8382208 -
Flags: review?(bugs)
Assignee | ||
Updated•10 years ago
|
Attachment #8382209 -
Flags: review?(bugs)
Assignee | ||
Updated•10 years ago
|
Attachment #8382210 -
Flags: review?(bugs)
Assignee | ||
Updated•10 years ago
|
Attachment #8382211 -
Flags: review?(bugs)
Assignee | ||
Updated•10 years ago
|
Attachment #8382212 -
Flags: review?(bugs)
Assignee | ||
Comment 28•10 years ago
|
||
Comment on attachment 8382214 [details] [diff] [review] part.16 Clean up dom::PointerEvent code Just cleaning up the PointerEvent implementation code for using same coding style with other DOM event classes.
Attachment #8382214 -
Flags: review?(bugs)
Assignee | ||
Comment 29•10 years ago
|
||
Comment on attachment 8382215 [details] [diff] [review] part.17 Rename dom/events/MutationEvent.h to dom/Events/InternalMutationEvent.h Renaming MutationEvent.h to InternalMutationEvent.h because it will conflict with renamed nsDOMMutationEvent.h. If we move this MutationEvent.h to widget/, we can distinguish the fine as mozilla/MutationEvent.h vs mozilla/dom/MutationEvent.h. But it may cause developers confused. Therefore, I like renaming as this patch better.
Attachment #8382215 -
Flags: review?(roc)
Attachment #8382215 -
Flags: review?(bugs)
Assignee | ||
Updated•10 years ago
|
Attachment #8382216 -
Flags: review?(bugs)
Comment 30•10 years ago
|
||
Comment on attachment 8382202 [details] [diff] [review] part.8 Rename nsDOMDeviceMotionEvent, nsDOMDeviceAcceleration and nsDOMDeviceRotationRate to mozilla::dom::DeviceMotionEvent, DeviceAcceleration and DeviceRotationRate > 'DeviceAcceleration': { >- 'nativeType': 'nsDOMDeviceAcceleration', >- 'headerFile': 'nsDOMDeviceMotionEvent.h', >+ 'headerFile': 'mozilla/dom/DeviceMotionEvent.h', >+ 'nativeType': 'mozilla::dom::DeviceAcceleration', > }, I think you don't need nativeType, since it is the default > > 'DeviceMotionEvent': { >- 'nativeType': 'nsDOMDeviceMotionEvent', >+ 'nativeType': 'mozilla::dom::DeviceMotionEvent', > }, I think you don't need this entry in .conf at all since nativeType is now the default one. > > 'DeviceRotationRate': { >- 'nativeType': 'nsDOMDeviceRotationRate', >- 'headerFile': 'nsDOMDeviceMotionEvent.h', >+ 'headerFile': 'mozilla/dom/DeviceMotionEvent.h', >+ 'nativeType': 'mozilla::dom::DeviceRotationRate', > }, I think you don't need nativeType, since it is the default
Attachment #8382202 -
Flags: review?(bugs) → review+
Comment 31•10 years ago
|
||
Comment on attachment 8382205 [details] [diff] [review] part.9 Rename nsDOMDragEvent to mozilla::dom::DragEvent > 'DragEvent': { >- 'nativeType': 'nsDOMDragEvent', >+ 'nativeType': 'mozilla::dom::DragEvent', > }, I think you can just remove the whole 'DragEvent' entry
Attachment #8382205 -
Flags: review?(bugs) → review+
Comment 32•10 years ago
|
||
Comment on attachment 8382206 [details] [diff] [review] part.10 Rename nsDOMFocusEvent to mozilla::dom::FocusEvent > 'FocusEvent': { >- 'nativeType': 'nsDOMFocusEvent', >+ 'nativeType': 'mozilla::dom::FocusEvent', > }, Again, I think the whole entry is now useless.
Attachment #8382206 -
Flags: review?(bugs) → review+
Comment 33•10 years ago
|
||
Comment on attachment 8382208 [details] [diff] [review] part.11 Rename nsDOMKeyboardEvent to mozilla::dom::KeyboardEvent > 'KeyboardEvent': { >- 'nativeType': 'nsDOMKeyboardEvent', >+ 'nativeType': 'mozilla::dom::KeyboardEvent', > }, Same thing here
Attachment #8382208 -
Flags: review?(bugs) → review+
Comment 34•10 years ago
|
||
Comment on attachment 8382209 [details] [diff] [review] part.12 Rename nsDOMMessageEvent to mozilla::dom::MessageEvent > 'MessageEvent': { >- 'nativeType': 'nsDOMMessageEvent', >+ 'nativeType': 'mozilla::dom::MessageEvent', > }, again the same thing
Attachment #8382209 -
Flags: review?(bugs) → review+
Comment 35•10 years ago
|
||
Comment on attachment 8382210 [details] [diff] [review] part.13 Rename nsDOMMouseScrollEvent to mozilla::dom::MouseScrollEvent > 'MouseScrollEvent': { >- 'nativeType': 'nsDOMMouseScrollEvent', >+ 'nativeType': 'mozilla::dom::MouseScrollEvent', > }, Again the same thing.
Attachment #8382210 -
Flags: review?(bugs) → review+
Comment 36•10 years ago
|
||
Comment on attachment 8382211 [details] [diff] [review] part.14 Rename nsDOMSimpleGestureEvent to mozilla::dom::SimpleGestureEvent > 'SimpleGestureEvent': { >- 'nativeType': 'nsDOMSimpleGestureEvent', >+ 'nativeType': 'mozilla::dom::SimpleGestureEvent', > }, ditto
Attachment #8382211 -
Flags: review?(bugs) → review+
Comment 37•10 years ago
|
||
Comment on attachment 8382212 [details] [diff] [review] part.15 Rename nsDOMMouseEvent to mozilla::dom::MouseEvent > 'MouseEvent': { >- 'nativeType': 'nsDOMMouseEvent', >+ 'nativeType': 'mozilla::dom::MouseEvent', > }, ditto
Attachment #8382212 -
Flags: review?(bugs) → review+
Updated•10 years ago
|
Attachment #8382214 -
Flags: review?(bugs) → review+
Comment 38•10 years ago
|
||
Comment on attachment 8382216 [details] [diff] [review] part.18 Rename nsDOMMutationEvent to mozilla::dom::MutationEvent > 'MutationEvent': { >- 'nativeType': 'nsDOMMutationEvent', >+ 'nativeType': 'mozilla::dom::MutationEvent', > }, Same case as elsewhere.
Attachment #8382216 -
Flags: review?(bugs) → review+
Updated•10 years ago
|
Attachment #8382215 -
Flags: review?(bugs) → review+
Attachment #8382215 -
Flags: review?(roc) → review+
Assignee | ||
Comment 39•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/39c3f1e13db0 https://hg.mozilla.org/integration/mozilla-inbound/rev/fd1c82a9839a https://hg.mozilla.org/integration/mozilla-inbound/rev/25906d620c44 https://hg.mozilla.org/integration/mozilla-inbound/rev/449bb2ddc4f1 https://hg.mozilla.org/integration/mozilla-inbound/rev/c2cd59ab0a41 https://hg.mozilla.org/integration/mozilla-inbound/rev/47dd7fd95629 https://hg.mozilla.org/integration/mozilla-inbound/rev/a9bc5bd8dc5a https://hg.mozilla.org/integration/mozilla-inbound/rev/b3ee0a6cbd75 https://hg.mozilla.org/integration/mozilla-inbound/rev/fa3367ca202d https://hg.mozilla.org/integration/mozilla-inbound/rev/cfc7b6e52e33 https://hg.mozilla.org/integration/mozilla-inbound/rev/dd8b08589d68
Comment 40•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/39c3f1e13db0 https://hg.mozilla.org/mozilla-central/rev/fd1c82a9839a https://hg.mozilla.org/mozilla-central/rev/25906d620c44 https://hg.mozilla.org/mozilla-central/rev/449bb2ddc4f1 https://hg.mozilla.org/mozilla-central/rev/c2cd59ab0a41 https://hg.mozilla.org/mozilla-central/rev/47dd7fd95629 https://hg.mozilla.org/mozilla-central/rev/a9bc5bd8dc5a https://hg.mozilla.org/mozilla-central/rev/b3ee0a6cbd75 https://hg.mozilla.org/mozilla-central/rev/fa3367ca202d https://hg.mozilla.org/mozilla-central/rev/cfc7b6e52e33 https://hg.mozilla.org/mozilla-central/rev/dd8b08589d68
Assignee | ||
Comment 41•10 years ago
|
||
According to your review, we can remove them now.
Attachment #8383557 -
Flags: review?(bugs)
Assignee | ||
Comment 42•10 years ago
|
||
Attachment #8383558 -
Flags: review?(bugs)
Assignee | ||
Comment 43•10 years ago
|
||
Attachment #8383559 -
Flags: review?(bugs)
Assignee | ||
Comment 44•10 years ago
|
||
Attachment #8383560 -
Flags: review?(bugs)
Assignee | ||
Comment 45•10 years ago
|
||
Attachment #8383561 -
Flags: review?(bugs)
Assignee | ||
Comment 46•10 years ago
|
||
Attachment #8383562 -
Flags: review?(bugs)
Assignee | ||
Comment 47•10 years ago
|
||
Attachment #8383563 -
Flags: review?(bugs)
Assignee | ||
Comment 48•10 years ago
|
||
Attachment #8383564 -
Flags: review?(bugs)
Assignee | ||
Comment 49•10 years ago
|
||
Although, nsDOMTimeEvent is under dom/smil, but this is the last derived event (except nsDOMTextEvent which will be removed by bug 974318) which is not in mozilla::dom namespace.
Attachment #8383567 -
Flags: review?(bugs)
Comment 50•10 years ago
|
||
Comment on attachment 8383557 [details] [diff] [review] part.19 Remove unnecessary items in Bindings.conf Just make sure to push these patches to try.
Attachment #8383557 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 51•10 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #50) > Comment on attachment 8383557 [details] [diff] [review] > part.19 Remove unnecessary items in Bindings.conf > > Just make sure to push these patches to try. https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=aeb468ff33c8 Yeah, I'm a big fan of tryserver.
Updated•10 years ago
|
Attachment #8383558 -
Flags: review?(bugs) → review+
Updated•10 years ago
|
Attachment #8383559 -
Flags: review?(bugs) → review+
Updated•10 years ago
|
Attachment #8383560 -
Flags: review?(bugs) → review+
Updated•10 years ago
|
Attachment #8383561 -
Flags: review?(bugs) → review+
Comment 52•10 years ago
|
||
Comment on attachment 8383562 [details] [diff] [review] part.24 Rename nsDOMNotifyPaintEvent to mozilla::dom::NotifyPaintEvent The comments in moz.build are odd, but not your fault.
Attachment #8383562 -
Flags: review?(bugs) → review+
Updated•10 years ago
|
Attachment #8383563 -
Flags: review?(bugs) → review+
Updated•10 years ago
|
Attachment #8383564 -
Flags: review?(bugs) → review+
Comment 53•10 years ago
|
||
Comment on attachment 8383567 [details] [diff] [review] part.27 Rename nsDOMTimeEvent to mozilla::dom::TimeEvent Hmm, one could actually implement TimeEvent using Event codegenerator (either the old one, if support for nsIDOMTimeEvent is needed, or the new one if webidl only is enough), I think. But we can do that change in a different bug.
Attachment #8383567 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 54•10 years ago
|
||
Thank you for the quick review! I'll check the TimeEvent next week. https://hg.mozilla.org/integration/mozilla-inbound/rev/283b3d83b335 https://hg.mozilla.org/integration/mozilla-inbound/rev/07ed9eef2248 https://hg.mozilla.org/integration/mozilla-inbound/rev/031dc7e7c074 https://hg.mozilla.org/integration/mozilla-inbound/rev/78f44c7e55f1 https://hg.mozilla.org/integration/mozilla-inbound/rev/cb9e4644b197 https://hg.mozilla.org/integration/mozilla-inbound/rev/e711121154fb https://hg.mozilla.org/integration/mozilla-inbound/rev/8c632b315211 https://hg.mozilla.org/integration/mozilla-inbound/rev/5db48f8fadb9 https://hg.mozilla.org/integration/mozilla-inbound/rev/4e1f613de77c
Assignee | ||
Comment 55•10 years ago
|
||
I'm working on renaming nsDOMEvent but it's a big patch. I'm going to post it next Monday. It may be the last patch of this bug. However, I wonder, should I rename NS_NewDOM*Event() too? If so, perhaps, mozilla::dom::Create*Event()? Although, it might be conflict with CreateEvent() in some places.
Flags: needinfo?(bugs)
Assignee | ||
Comment 56•10 years ago
|
||
mozilla::dom::CreateNew*Event()?
Comment 57•10 years ago
|
||
If we need a factor method at all, how about mozilla::dom::New*Event ?
Assignee | ||
Comment 58•10 years ago
|
||
Sounds good to me. How about Smaug?
Comment 59•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/283b3d83b335 https://hg.mozilla.org/mozilla-central/rev/07ed9eef2248 https://hg.mozilla.org/mozilla-central/rev/031dc7e7c074 https://hg.mozilla.org/mozilla-central/rev/78f44c7e55f1 https://hg.mozilla.org/mozilla-central/rev/cb9e4644b197 https://hg.mozilla.org/mozilla-central/rev/e711121154fb https://hg.mozilla.org/mozilla-central/rev/8c632b315211 https://hg.mozilla.org/mozilla-central/rev/5db48f8fadb9 https://hg.mozilla.org/mozilla-central/rev/4e1f613de77c
Comment 60•10 years ago
|
||
Please don't rename NS_New* since I think we should make all the events to use similar way to construct what codegenerated events, at least, support. FooEventInit init; init.bubbles = true; ... nsRefPtr<FooEvent> event = FooEvent::Construct(someTarget, NS_LITERAL_STRING("footype"), init); Once everything uses that, we could perhaps just remove NS_New*
Flags: needinfo?(bugs)
Assignee | ||
Comment 61•10 years ago
|
||
Okay. I'll keep them.
Assignee | ||
Comment 62•10 years ago
|
||
The last patch of this bug.
Attachment #8385189 -
Flags: review?(bugs)
Updated•10 years ago
|
Attachment #8385189 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 63•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8b872592e441
Whiteboard: [leave open]
Comment 64•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8b872592e441
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in
before you can comment on or make changes to this bug.
Description
•