Status

()

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

Trunk
mozilla6
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

I'll do this after part.6 for bug 519972 landed.
Posted patch Patch v1.0 (obsolete) — Splinter Review
There are two plugin events:
1. NS_PLUGIN_EVENT, this is used on Windows.
2. NS_NON_RETARGETED_PLUGIN_EVENT, this is used on Mac and not retargeted to focused presShell.

And there are two kinds of the events, one is for key or IME, i.e., text input related events. The other is for focus event handling on Mac.

I don't think the latter should be dispatched by plugin input event because it can confuse the developers.

I sorted out the plugin events by this patch.

I created two events: One is NS_PLUGIN_INPUT_EVENT_COURIER, this is for input events for plugins. The other is NS_PLUGIN_FOCUS_EVENT_COURIER, this is for focus events for plugins.

And I create nsPluginEventCourier which is a subclass of nsGUIEvent. It has retargetNeeded member. It indicates whether the event should be retargeted to focused presShell or not.

I think that "courier" (or "deliverer"?) should be included in their names because it documents their job in their names.

How do you think, roc?
Attachment #531238 - Flags: review?(roc)
Courier isn't the right word here. It's not used for this kind of thing. "Carrier" would be a better word. However, I actually think the old names were the best: NS_PLUGIN_INPUT_EVENT and NS_PLUGIN_FOCUS_EVENT sound good.

"retargetNeeded" should probably be more clear: e.g. "retargetToFocusedDocument".

The rest of the approach sounds OK.
Okay, thank you. I'll update the patch soon.
Posted patch Patch v2.0 (obsolete) — Splinter Review
Attachment #531238 - Attachment is obsolete: true
Attachment #531238 - Flags: review?(roc)
Attachment #531247 - Flags: review?(roc)
Summary: NS_PLUGIN_EVENT should be renamed NS_PLUGIN_INPUT_EVENT → Sort out NS_PLUGIN_EVENT
Comment on attachment 531247 [details] [diff] [review]
Patch v2.0

Review of attachment 531247 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsObjectFrame.cpp
@@ +6899,5 @@
>      InitializeNPCocoaEvent(&cocoaEvent);
>      cocoaEvent.type = NPCocoaEventWindowFocusChanged;
>      cocoaEvent.data.focus.hasFocus = NS_NPAPI_CocoaWindowIsMain(cocoaTopLevelWindow);
>      pluginEvent.pluginEvent = &cocoaEvent;
> +    pluginEvent.retargetToFocusedDocument = PR_FALSE;

Not needed because the default is false.

::: widget/public/nsGUIEvent.h
@@ +1541,5 @@
> +  {
> +  }
> +
> +  // If TRUE, this event needs to be retargeted to focused document.
> +  // Otherwise, never retargeted.

Add "Defaults to false."

@@ +1634,5 @@
>  #define NS_IS_PLUGIN_EVENT(evnt) \
> +       (((evnt)->message == NS_PLUGIN_INPUT_EVENT) || \
> +        ((evnt)->message == NS_PLUGIN_FOCUS_EVENT))
> +
> +#define NS_IS_RETARGET_NEEDED_PLUGIN_EVENT(evnt) \

NS_IS_RETARGETED_PLUGIN_EVENT
Attachment #531247 - Flags: review?(roc) → review+
Steven should know this change.
http://hg.mozilla.org/mozilla-central/rev/b46f3c4c5059
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
You need to log in before you can comment on or make changes to this bug.