Closed
Bug 1455055
Opened 6 years ago
Closed 6 years ago
Convert nsIDOMEventListener to taking Event, not nsIDOMEvent
Categories
(Core :: DOM: Events, defect)
Core
DOM: Events
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
Attachments
(7 files)
82.15 KB,
patch
|
masayuki
:
review+
|
Details | Diff | Splinter Review |
20.37 KB,
patch
|
masayuki
:
review+
|
Details | Diff | Splinter Review |
22.52 KB,
patch
|
masayuki
:
review+
|
Details | Diff | Splinter Review |
6.46 KB,
patch
|
masayuki
:
review+
|
Details | Diff | Splinter Review |
11.73 KB,
patch
|
masayuki
:
review+
|
Details | Diff | Splinter Review |
27.86 KB,
patch
|
masayuki
:
review+
|
Details | Diff | Splinter Review |
16.26 KB,
patch
|
masayuki
:
review+
|
Details | Diff | Splinter Review |
Sadly, this is going to end up as a biggish patch if it's going to compile at all...
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 1•6 years ago
|
||
Jorg, you'll need a comm-central version of this too. Should be pretty straightforward once you see the changes I'm making in m-c.
Assignee | ||
Comment 2•6 years ago
|
||
This does no cleanup other than what's needed to compile. Cleanup coming up in later patches. MozReview-Commit-ID: 3sOnkj71n09
Attachment #8969245 -
Flags: review?(bugs)
Assignee | ||
Comment 3•6 years ago
|
||
They take Event now so we can simplify some things. MozReview-Commit-ID: HUbn7QWqRAQ
Attachment #8969246 -
Flags: review?(bugs)
Assignee | ||
Comment 4•6 years ago
|
||
MozReview-Commit-ID: 7Irm8aAmeIt
Attachment #8969247 -
Flags: review?(bugs)
Assignee | ||
Comment 5•6 years ago
|
||
MozReview-Commit-ID: B3ez0ESo21g
Attachment #8969248 -
Flags: review?(bugs)
Assignee | ||
Comment 6•6 years ago
|
||
MozReview-Commit-ID: 1DOlnGP4Sri
Attachment #8969249 -
Flags: review?(bugs)
Assignee | ||
Comment 7•6 years ago
|
||
MozReview-Commit-ID: Ht7HQEhVS8E
Attachment #8969250 -
Flags: review?(bugs)
Assignee | ||
Comment 8•6 years ago
|
||
MozReview-Commit-ID: LezJYKK74H5
Attachment #8969251 -
Flags: review?(bugs)
Assignee | ||
Updated•6 years ago
|
Attachment #8969245 -
Flags: review?(bugs) → review?(masayuki)
Assignee | ||
Updated•6 years ago
|
Attachment #8969246 -
Flags: review?(bugs) → review?(masayuki)
Assignee | ||
Updated•6 years ago
|
Attachment #8969247 -
Flags: review?(bugs) → review?(masayuki)
Assignee | ||
Updated•6 years ago
|
Attachment #8969248 -
Flags: review?(bugs) → review?(masayuki)
Assignee | ||
Updated•6 years ago
|
Attachment #8969249 -
Flags: review?(bugs) → review?(masayuki)
Assignee | ||
Updated•6 years ago
|
Attachment #8969250 -
Flags: review?(bugs) → review?(masayuki)
Assignee | ||
Updated•6 years ago
|
Attachment #8969251 -
Flags: review?(bugs) → review?(masayuki)
Comment 9•6 years ago
|
||
Comment on attachment 8969245 [details] [diff] [review] part 1. Convert nsIDOMEventListener to taking an Event, not an nsIDOMEvent Review of attachment 8969245 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/interfaces/events/nsIDOMEventListener.idl @@ +4,5 @@ > * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > > #include "domstubs.idl" > > +webidl Event; Wow, I didn't know that we can do this!
Attachment #8969245 -
Flags: review?(masayuki) → review+
Updated•6 years ago
|
Attachment #8969246 -
Flags: review?(masayuki) → review+
Assignee | ||
Comment 10•6 years ago
|
||
> Wow, I didn't know that we can do this! We couldn't until two days ago. ;) See https://groups.google.com/d/msg/mozilla.dev.platform/uwfhoVi7r98/oypB1UrACQAJ
Updated•6 years ago
|
Attachment #8969247 -
Flags: review?(masayuki) → review+
Updated•6 years ago
|
Attachment #8969248 -
Flags: review?(masayuki) → review+
Comment 11•6 years ago
|
||
Comment on attachment 8969249 [details] [diff] [review] part 5. Clean up HandleEvent implementations in dom/xbl Review of attachment 8969249 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/xbl/nsXBLEventHandler.cpp @@ +36,5 @@ > return NS_ERROR_FAILURE; > > uint8_t phase = mProtoHandler->GetPhase(); > if (phase == NS_PHASE_TARGET) { > + if (aEvent->EventPhase() != EventBinding::AT_TARGET) nit: Could you wrap one line block with {} in this kind of chance? @@ +126,5 @@ > if (count == 0) > return NS_ERROR_FAILURE; > > if (mPhase == NS_PHASE_TARGET) { > + if (aEvent->EventPhase() != EventBinding::AT_TARGET) nit: same here.
Attachment #8969249 -
Flags: review?(masayuki) → review+
Comment 12•6 years ago
|
||
Comment on attachment 8969250 [details] [diff] [review] part 6. Clean up HandleEvent implementations in dom Review of attachment 8969250 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/plugins/base/nsPluginInstanceOwner.h @@ +99,5 @@ > > // nsIDOMEventListener interfaces > NS_DECL_NSIDOMEVENTLISTENER > > + nsresult ProcessMouseDown(mozilla::dom::Event* aKeyEvent); nit: aMouseEvent?
Attachment #8969250 -
Flags: review?(masayuki) → review+
Comment 13•6 years ago
|
||
Comment on attachment 8969251 [details] [diff] [review] part 7. Clean up remaining HandleEvent implementations Review of attachment 8969251 [details] [diff] [review]: ----------------------------------------------------------------- Thank you very much! These patches are really nice especially for Quantum Flow. We should keep struggling with XPCOM specific overhead.
Attachment #8969251 -
Flags: review?(masayuki) → review+
Assignee | ||
Comment 14•6 years ago
|
||
> nit: Could you wrap one line block with {} in this kind of chance? Done, both places. > nit: aMouseEvent? Done. And yes, more XPCOM removal coming. ;)
Comment 15•6 years ago
|
||
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6567a1d09c39 part 1. Convert nsIDOMEventListener to taking an Event, not an nsIDOMEvent. r=masayuki https://hg.mozilla.org/integration/mozilla-inbound/rev/ee04ee4104b6 part 2. Clean up HandleEvent implementations in layout/xul. r=masayuki https://hg.mozilla.org/integration/mozilla-inbound/rev/21dc0a589533 part 3. Clean up HandleEvent implementations in layout. r=masayuki https://hg.mozilla.org/integration/mozilla-inbound/rev/a8a18e514b3d part 4. Clean up HandleEvent implementations in accessible. r=masayuki https://hg.mozilla.org/integration/mozilla-inbound/rev/3f6f6c68e211 part 5. Clean up HandleEvent implementations in dom/xbl. r=masayuki https://hg.mozilla.org/integration/mozilla-inbound/rev/491b4ebcc48a part 6. Clean up HandleEvent implementations in dom. r=masayuki https://hg.mozilla.org/integration/mozilla-inbound/rev/966c8536f1bd part 7. Clean up remaining HandleEvent implementations. r=masayuki
Comment 16•6 years ago
|
||
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #10) > > Wow, I didn't know that we can do this! > > We couldn't until two days ago. ;) See > https://groups.google.com/d/msg/mozilla.dev.platform/uwfhoVi7r98/oypB1UrACQAJ Thanks!
Comment 17•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6567a1d09c39 https://hg.mozilla.org/mozilla-central/rev/ee04ee4104b6 https://hg.mozilla.org/mozilla-central/rev/21dc0a589533 https://hg.mozilla.org/mozilla-central/rev/a8a18e514b3d https://hg.mozilla.org/mozilla-central/rev/3f6f6c68e211 https://hg.mozilla.org/mozilla-central/rev/491b4ebcc48a https://hg.mozilla.org/mozilla-central/rev/966c8536f1bd
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in
before you can comment on or make changes to this bug.
Description
•