384975 - input event dispatching for X11 windowless plugins

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Karl Tomlinson (:karlt)]

Attachment: input event dispatching

Mouse, key, focus, and enter/leave events are provided in XEvents
through NPP_HandleEvent.  The mouse events have x and y relative
to the plugin rectangle origin.  This seemed the most useful
information to me, but is not the same as what is provided on
Windows.

(On Windows the mouse events are relative to the containing window.  This requires working with one offset for drawing and a different offset for mouse events.  See bug 135737.  I'd rather avoid this.)

As the XEvents sent to the plugin are synthesized and there is not
a native window corresponding to the plugin rectangle, some of the
members of the XEvent structures are not set to their normal
Xserver values.  e.g. Key events don't have a position.  However
the information should be complete enough.

This patch requires the patch in 384965 and (at least) some of the patch in 384845.

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: input event dispatching 2

Added #ifdef MOZ_WIDGET_GTK2 where appropriate.
Using NS_STATIC_CAST.

roc can you review the code, please?
jst can you confirm that you are happy with the plugin API, please?

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+  if(anEvent.isAlt) state |= Mod1Mask;
+  if(anEvent.isMeta) state |= Mod4Mask;

Are these always the right masks? Check out the modifier-related code in the patch in bug 377478.

+#ifdef MOZ_X11
+  // this code supports windowless plugins

Could we break this out into a helper function?

+        // XXX Non-translation transformations will be interesting.

Take this out ... nsLayoutUtils::GetEventCoordinatesRelativeTo should be fine with this.

+          } break;

Put this on its own line.

Comment 3

[Karl Tomlinson (:karlt)]

(In reply to comment #2)
> +  if(anEvent.isAlt) state |= Mod1Mask;
> +  if(anEvent.isMeta) state |= Mod4Mask;
> 
> Are these always the right masks? Check out the modifier-related code in the
> patch in bug 377478.

These invert processing in nsWindow::OnMotionNotifyEvent, nsCommonWidget::InitButtonEvent, and nsCommonWidget::InitKeyEvent, and so they are correct in that they recover the information stored in the nsInputEvent.

As to the question of whether nsCommonWidget and nsWindow have it right:

Gtk always treats alt as Mod1 in gtk_accelerator_parse

          else if (len >= 5 && is_alt (accelerator))
            {
              accelerator += 5;
              len -= 5;
              mods |= GDK_MOD1_MASK;
            }

and in gtk_accelerator_name

  static const gchar text_mod1[] = "<Alt>";

  ...

  if (accelerator_mods & GDK_MOD1_MASK)
    {
      strcpy (accelerator + l, text_mod1);
      l += sizeof (text_mod1) - 1;
    }

so I guess the isAlt Mod1 association is safe.

As for isMeta, Meta and Alt are usually bound to the same modifier so I guess isMeta really means something like isSuperOrHyper, in which case modifier key detection like that in bug 377478 would be best.  However, I think this modifier is often associated with the window manager so it probably should not be interpreted by us anyway.  (nsWindow::OnMotionNotifyEvent doesn't set isMeta at all, but nsCommonWidget does.)

> 
> +#ifdef MOZ_X11
> +  // this code supports windowless plugins
> 
> Could we break this out into a helper function?

I'll leave this as it stands as there wouldn't be must left of ProcessEvent if we broke this out.

> 
> +        // XXX Non-translation transformations will be interesting.
> 
> Take this out ... nsLayoutUtils::GetEventCoordinatesRelativeTo should be fine
> with this.

Yep.

> 
> +          } break;
> 
> Put this on its own line.

I'm happy to do that as it makes sense to me to.  The style above was based on 
http://www.mozilla.org/hacking/mozilla-style-guide.html.

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: input event dispatching 3

Removed non-translation comment as requested.
s/} break/}\n break/

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Checked those in.