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

ASSIGNED
Assigned to

Status

()

Core
Widget: Gtk
P3
normal
ASSIGNED
7 years ago
a year ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: tpi:+)

Attachments

(1 attachment)

Created attachment 548691 [details] [diff] [review]
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.
Attachment #548691 - Flags: feedback?(hiikezoe)
Hello, Jeff. Do you know someone to have some advice for this bug?
(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.
Hmm, I don't like separate it. How about is GetContext() renamed to GetContextForCurrentState() or something?
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).
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

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

Comment 7

2 years ago
(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?
Flags: needinfo?(jmott)

Updated

2 years ago
Flags: needinfo?(jmott)

Comment 8

2 years ago
Never did, sorry, haven't worked on this in years - removing myself from the bug.
Priority: -- → P3
Whiteboard: tpi:+
You need to log in before you can comment on or make changes to this bug.