674456 - Shouldn't pass key events to *our* IM context when a windowless plugin has focus

Status: RESOLVED WONTFIX

(Core :: Widget: Gtk, defect, P3)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch

On GTK, plugins should have its owning IM context even if it's in windowless mode because IM related events are not notified by GdkEvent, they're signals of GtkIMContext, so, we cannot dispatch any native events to plugins by NS_PLUGIN_INPUT_EVENT.

However, I'm not familiar with GTK plugins and OOPP. I'm not sure whether windowless plugins can do it. I need feedback about this before review.

# NOTE: currently, flash player don't handle IM events in windowless mode.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hello, Jeff. Do you know someone to have some advice for this bug?

Comment 2

[Hiroyuki Ikezoe (:hiro)]

(In reply to comment #0)
> 
> On GTK, plugins should have its owning IM context even if it's in windowless

Basically I agree with you. But the check in nsGtkIMModule::GetContext() is a bit tricky because the check can not been seen outside of the method (I mean at the caller side) and the name, GetContext(), does not tell us the check is inside. 

I prefer to create a new method like nsGtkIMModule::theWidgetHasOwnContext() (actually this name is very awful, we need a more appropriate name here) and call the new method before GetContext(). So at the caller side, do nothing in gecko side if the widget (I am not sure widget is a good term here) has its own context. Then it will be easily to read what's going on there.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, I don't like separate it. How about is GetContext() renamed to GetContextForCurrentState() or something?

Comment 4

[Hiroyuki Ikezoe (:hiro)]

The main problem of reuse GetContext() is that it can not be distinguished from failure of the getter method. If we reuse the method, I propose GetOurOwnContext() or GetContextInGecko() or ... Anyway the name should represent the limitation (In this case we do not get the IM context in plugins).

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

You don't need the failure case. If it failed to allocate one of the contexts, the process must have been aborted by gtk. If some methods were called after the related window was destroyed, it would return NULL but it's very rare case. See bug 335028.

Let me think about the name. But I don't think that it might return non-our context even if I didn't design it. Therefore, I think that GetCurrentContext() or something which doesn't contain "our" is enough...

Comment 6

[Jeff Mott]

Sorry I really don't know much about Linux - I'll see if I can find someone to comment.

Comment 7

[Wayne Mery (:wsmwk)]

(In reply to Jeff Mott from comment #6)
> Sorry I really don't know much about Linux - I'll see if I can find someone
> to comment.

no luck?

Comment 8

[Jeff Mott]

Never did, sorry, haven't worked on this in years - removing myself from the bug.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

We shouldn't do anything for plugins anymore.