Last Comment Bug 663879 - Kill AddEventListenerByIID/RemoveEventListenerByIID from extensions
: Kill AddEventListenerByIID/RemoveEventListenerByIID from extensions
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Jonas Sicking (:sicking)
:
Mentors:
Depends on:
Blocks: 661297
  Show dependency treegraph
 
Reported: 2011-06-13 10:31 PDT by Jonas Sicking (:sicking)
Modified: 2011-06-28 18:55 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch to fix (20.29 KB, patch)
2011-06-13 10:31 PDT, Jonas Sicking (:sicking)
no flags Details | Diff | Review
Patch to fix (22.04 KB, patch)
2011-06-13 12:02 PDT, Jonas Sicking (:sicking)
bugs: review+
Details | Diff | Review

Description Jonas Sicking (:sicking) 2011-06-13 10:31:29 PDT
Created attachment 538949 [details] [diff] [review]
Patch to fix

I checked all code paths called by HandleEvent and they always QI and bail if the event doesn't implement the desired interface.

Still running this through tryserver, as it seems like some of this code is platform dependent, so not asking for review yet.
Comment 1 Jonas Sicking (:sicking) 2011-06-13 12:02:33 PDT
Created attachment 538974 [details] [diff] [review]
Patch to fix

The previous patch lacked adjustments to the QI implementations. This one seems to be passing tryserver.
Comment 2 Olli Pettay [:smaug] 2011-06-13 12:16:40 PDT
Comment on attachment 538974 [details] [diff] [review]
Patch to fix


> NS_IMETHODIMP mozInlineSpellChecker::HandleEvent(nsIDOMEvent* aEvent)
> {
>+  nsAutoString eventType;
>+  aEvent->GetType(eventType);
>+
>+  if (eventType.EqualsLiteral("blur"))
>+    return Blur(aEvent);
>+  if (eventType.EqualsLiteral("click"))
>+    return MouseClick(aEvent);
>+  if (eventType.EqualsLiteral("keypress"))
>+    return KeyPress(aEvent);

Coding style is:
if (expr) {
  stmt;
}


>+  if (eventType.EqualsLiteral("mousedown"))
>+    return MouseDown(aEvent);
>+  if (eventType.EqualsLiteral("mouseup"))
>+    return MouseUp(aEvent);
>+  if (eventType.EqualsLiteral("mousemove"))
>+    return MouseMove(aEvent);

if (expr) {
  stmt;
}
Comment 3 Jonas Sicking (:sicking) 2011-06-24 17:11:53 PDT
Checked in to m-c:
http://hg.mozilla.org/mozilla-central/rev/967b254211be

Thanks for the review!
Comment 4 Jonas Sicking (:sicking) 2011-06-24 17:43:50 PDT
Aww, crap, this wasn't the one I intended to land. This depends on bug 663461.
Comment 5 Jonas Sicking (:sicking) 2011-06-28 08:15:58 PDT
Landed http://hg.mozilla.org/integration/mozilla-inbound/rev/ab2e92a211e1

Thanks for review!
Comment 6 Joe Drew (not getting mail) 2011-06-28 18:55:22 PDT
http://hg.mozilla.org/mozilla-central/rev/ab2e92a211e1

Note You need to log in before you can comment on or make changes to this bug.