Created attachment 540500 [details] [diff] [review] Part 1: Remove ByIID code from plugininstanceowner Josh is currently working on a big rewrite of plugin stuff (bug 90268). If that finishes first I'll produce patches on top of that. Otherwise I'll do patches on top of m-c, and then help Josh merge those patches. So not asking for review on these patches yet. Just attaching for safekeeping
Created attachment 540502 [details] [diff] [review] Part 2: Remove from ByIID code from nsPluginDOMContextMenuListener
Created attachment 542566 [details] [diff] [review] Part 1: Remove ByIID code from plugininstanceowner So this is getting close to be the only remaining *EventListenerByIID user, so asking for review. Josh, find me on irc and we'll figure out a plan for merging with your other work.
Created attachment 542567 [details] [diff] [review] Part 2: Remove from ByIID code from nsPluginDOMContextMenuListener
Hey Jonas - go ahead and land this whenever you want. Don't wait for me.
Comment on attachment 542566 [details] [diff] [review] Part 1: Remove ByIID code from plugininstanceowner Oops, thought I had flagged you for reviewer, but apparently not
Comment on attachment 542566 [details] [diff] [review] Part 1: Remove ByIID code from plugininstanceowner Review of attachment 542566 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/plugins/base/nsPluginInstanceOwner.cpp @@ +1815,3 @@ > if (nsEventStatus_eConsumeNoDefault == rv) { > aMouseEvent->PreventDefault(); > + // We didn't used to do this when This looks like an unfinished comment. @@ +1834,5 @@ > + mContentFocused = PR_TRUE; > + return DispatchFocusToPlugin(aEvent); > + } > + if (eventType.EqualsLiteral("blur")) { > + mContentFocused = PR_TRUE; This is incorrect - mContentFocused should be set to PR_FALSE here. Copy and paste error? @@ +1851,5 @@ > + return NS_OK; > + } > + return DispatchMouseToPlugin(aEvent); > + } > + if (eventType.EqualsLiteral("mousemove") || It looks like you didn't retain most of the logic in nsPluginInstanceOwner::MouseMove, which you removed. Did you determine that it is unnecessary for some reason or did you just forget to move it?
> ::: dom/plugins/base/nsPluginInstanceOwner.cpp > @@ +1815,3 @@ > > if (nsEventStatus_eConsumeNoDefault == rv) { > > aMouseEvent->PreventDefault(); > > + // We didn't used to do this when > > This looks like an unfinished comment. Removed the comment, see below. > @@ +1834,5 @@ > > + mContentFocused = PR_TRUE; > > + return DispatchFocusToPlugin(aEvent); > > + } > > + if (eventType.EqualsLiteral("blur")) { > > + mContentFocused = PR_TRUE; > > This is incorrect - mContentFocused should be set to PR_FALSE here. Copy and > paste error? Good catch! Fixed! > @@ +1851,5 @@ > > + return NS_OK; > > + } > > + return DispatchMouseToPlugin(aEvent); > > + } > > + if (eventType.EqualsLiteral("mousemove") || > > It looks like you didn't retain most of the logic in > nsPluginInstanceOwner::MouseMove, which you removed. Did you determine that > it is unnecessary for some reason or did you just forget to move it? It looked like nsPluginInstanceOwner::MouseMove did exactly the same thing as nsPluginInstanceOwner::DispatchMouseToPlugin. With the only exception that MouseMove didn't call StopPropagation which I suspect was just an omission (that's what the comment above was going to be about). Did I miss some logic in nsPluginInstanceOwner::MouseMove that isn't in nsPluginInstanceOwner::DispatchMouseToPlugin?
Created attachment 551348 [details] [diff] [review] Part 1: Remove ByIID code from plugininstanceowner
Nice catch on the MouseMove vs. DispatchMouseToPlugin thing. Better not to duplicate that.
Checked in to inbound. Thanks for the super quick rereview! http://hg.mozilla.org/integration/mozilla-inbound/rev/ec15be691f62 http://hg.mozilla.org/integration/mozilla-inbound/rev/196eadbe49de
Comment on attachment 551348 [details] [diff] [review] Part 1: Remove ByIID code from plugininstanceowner >+ (ev = (EventRecord*)(((nsGUIEvent*)theEvent)->pluginEvent)) && Ugh.
http://hg.mozilla.org/mozilla-central/rev/ec15be691f62 http://hg.mozilla.org/mozilla-central/rev/196eadbe49de
or
