Closed Bug 661297 Opened 13 years ago Closed 13 years ago

Kill AddEventListenerByIID/RemoveEventListenerByIID

Categories

(Core :: DOM: Events, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla8

People

(Reporter: sicking, Assigned: sicking)

References

Details

Attachments

(2 files)

We should make users use the standard AddEventListener instead and register for the events they want. The plan of attack here goes something like this: 1. Remove the Add/RemoveEventListenerByIID users a few at a time until none are left. 2. Remove the Add/RemoveEventListenerByIID API. 3. Cleanup internal code that deals with IID listeners. I'll file separate bugs for step one and attach patches there so that we can space them out. There is a decent chance of regressions here so I'd rather land the removals a few at a time spaced out over multiple days as to make bisecting easier. Steps 2 and 3 can probably happen in this bug though.
Attached patch Part 1: Remove API part (deleted) — Splinter Review
Assignee: nobody → jonas
Attachment #540541 - Flags: review?(Olli.Pettay)
Attachment #540541 - Attachment description: Remove API part → Part 1: Remove API part
Comment on attachment 540546 [details] [diff] [review] Part 2: Remove implementation of by-iid listeners >--- a/content/events/src/nsEventListenerManager.cpp >+++ b/content/events/src/nsEventListenerManager.cpp >@@ -1206,53 +1047,23 @@ nsEventListenerManager::GetListenerInfo( >+ const nsDependentSubstring& eventType = >+ Substring(nsDependentAtomString(ls.mTypeAtom), 2); >+ nsRefPtr<nsEventListenerInfo> info = >+ new nsEventListenerInfo(eventType, ls.mListener, capturing, >+ allowsUntrusted, systemGroup); >+ NS_ENSURE_TRUE(info, NS_ERROR_OUT_OF_MEMORY); You can remove this line, while you're here.
Comment on attachment 540541 [details] [diff] [review] Part 1: Remove API part Splitting the patches makes reviewing a bit harder :/
Attachment #540541 - Flags: review?(Olli.Pettay) → review+
Comment on attachment 540546 [details] [diff] [review] Part 2: Remove implementation of by-iid listeners >+ const nsDependentSubstring& eventType = >+ Substring(nsDependentAtomString(ls.mTypeAtom), 2); It is guaranteed to have mTypeAtom, right?
Attachment #540546 - Flags: review?(Olli.Pettay) → review+
Yep. All entry points now specify a name in one way or another.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
Landed followup to remove reference to a now-missing Makefile from toolkit-makefiles.sh (now-missing because of the "dom/public" deletion in c1c58c2bf9df): http://hg.mozilla.org/integration/mozilla-inbound/rev/1127ccbf8f4e This followup fixes the new build warning: { creating dom/public/coreEvents/Makefile can't read /builds/slave/m-cen-lnx64-dbg/build/dom/public/coreEvents/Makefile.in: No such file or directory } http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1313167929.1313174225.25821.gz
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: