[Gtk] Need to investigate if IMEs on Linux support vertical writing mode and how do we tells it to IME

RESOLVED FIXED in Firefox 41

Status

()

Core
Widget: Gtk
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

(Blocks: 2 bugs, {inputmethod})

Trunk
mozilla41
Unspecified
Linux
inputmethod
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(3 attachments)

I'm not sure if IMEs on Linux support vertical writing mode. If they support it, we need to tell the focused editor's text direction to IME.

I've not found any information about this in the document of GtkIMContext:
https://developer.gnome.org/gtk3/unstable/GtkIMContext.html

Even if it's not supported, I guess that we need to move candidate window to top-right corner of caret rect when focus is in vertical writing mode editor.
No longer blocks: 1076657
Fctix's author told me that he doesn't know any hint from the system about whether the text should be vertical or not.
(In reply to Xidorn Quan [:xidorn] (UTC+11) from comment #1)
> Fctix's author told me that he doesn't know any hint from the system about
> whether the text should be vertical or not.

Thanks. I and Kato-san also think that there is no API for this.

But probably, we need to add a hack at calling gtk_im_context_set_cursor_location().
https://developer.gnome.org/gtk3/unstable/GtkIMContext.html#gtk-im-context-set-cursor-location

I guess that IME candidate window must overlaps with composition string if the caret is not positioned at the end of the composition string.
http://mxr.mozilla.org/mozilla-central/source/widget/gtk/nsGtkIMModule.cpp?rev=4631a7474d8a#1337
TODO (feature plan)
- We should suggest GTK team (or RedHat's SCIM/ibus team) to add API for vertical layout

Updated

2 years ago
No longer blocks: 1099032

Updated

2 years ago
Okay, I'll create patches to move candidate window position to botton-left of target clause rect. I think that it's enough for now. With that approach, candidate window will never overlap with the target clause (focused clause).
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Ah, or, should I file a new bug for temporarily fix?
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) (offline: 5/2 - 5/6) from comment #5)
> Ah, or, should I file a new bug for temporarily fix?

I think it'd be fine to restrict the scope of this bug (make it something like "Position IME candidate window appropriately in vertical writing mode on Linux"?) so it just covers the fix we can actually do for now, given the limitations of the platform we're running on.

We'll do the best we can on the platform as it is today; and when the platform gains new capabilities, that's the time to file new bugs about supporting the new functionality.
OS: Windows 8.1 → Linux
Hardware: x86_64 → Unspecified
add ibus's bug
Created attachment 8617254 [details] [diff] [review]
part.1 nsGtkIMModule should cache selection

I'll request reviews after trysever is back and the patches are tested on it.
Created attachment 8617256 [details] [diff] [review]
part.2 nsGtkIMModule should set candidiate window position to bottom left of the target clause in vertical writing mode
Created attachment 8617257 [details] [diff] [review]
part.3 nsGtkIMModule should adjust candidate window position when layout is changed
Comment on attachment 8617254 [details] [diff] [review]
part.1 nsGtkIMModule should cache selection

First, let's cache selection range and writing mode in nsGtkIMModule.
Attachment #8617254 - Flags: review?(m_kato)
Comment on attachment 8617256 [details] [diff] [review]
part.2 nsGtkIMModule should set candidiate window position to bottom left of the target clause in vertical writing mode

Next, this is what we need to do here.

Cache target clause range at dispatching NS_COMPOSITION_CHANGE and if it's in vertical writing mode, let's set "fake" caret rect which is very tall. Then, IME must avoid to use the area.

As far as I tested on iBus, the tall caret doesn't increase font size of candidate window, fortunately.
Attachment #8617256 - Flags: review?(m_kato)
Comment on attachment 8617257 [details] [diff] [review]
part.3 nsGtkIMModule should adjust candidate window position when layout is changed

Additional fixes.

1. Use position change notification.
2. Cache the target clause range *before* dispatching NS_COMPOSITION_CHANGE.
Attachment #8617257 - Flags: review?(m_kato)
Comment on attachment 8617254 [details] [diff] [review]
part.1 nsGtkIMModule should cache selection

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

::: widget/gtk/nsGtkIMModule.cpp
@@ +80,5 @@
> +public:
> +  explicit GetWritingModeName(const WritingMode& aWritingMode)
> +  {
> +    if (!aWritingMode.IsVertical()) {
> +      Assign("Horizontal");

Use AssignLiteral

@@ +84,5 @@
> +      Assign("Horizontal");
> +      return;
> +    }
> +    if (aWritingMode.IsVerticalLR()) {
> +      Assign("Vertical (LTR)");

Use AssignLiteral

@@ +87,5 @@
> +    if (aWritingMode.IsVerticalLR()) {
> +      Assign("Vertical (LTR)");
> +      return;
> +    }
> +    Assign("Vertical (RTL)");

Use AssignLiteral

@@ +739,5 @@
> +    mSelection.mOffset = aIMENotification.mSelectionChangeData.mOffset;
> +    mSelection.mLength = aIMENotification.mSelectionChangeData.mLength;
> +    mSelection.mWritingMode =
> +        aIMENotification.mSelectionChangeData.GetWritingMode();
> +

3 parameters are all member of mSelection.  So I think you should add assign or init method to use like mSelection.Assign(aIMENotification).

::: widget/gtk/nsGtkIMModule.h
@@ +191,5 @@
> +            : mOffset(UINT32_MAX)
> +            , mLength(UINT32_MAX)
> +        {
> +        }
> +

need Assign method or operator = (IMENotification)?
Attachment #8617254 - Flags: review?(m_kato) → review+

Updated

2 years ago
Attachment #8617256 - Flags: review?(m_kato) → review+

Updated

2 years ago
Attachment #8617257 - Flags: review?(m_kato) → review+
(In reply to Makoto Kato (:m_kato) from comment #14)
> need Assign method or operator = (IMENotification)?

ah, operator = is no good.  we should use init or assign

Comment 16

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/caa60ea601b2
https://hg.mozilla.org/integration/mozilla-inbound/rev/1810110176bf
https://hg.mozilla.org/integration/mozilla-inbound/rev/c6fb4870676c
https://hg.mozilla.org/mozilla-central/rev/caa60ea601b2
https://hg.mozilla.org/mozilla-central/rev/1810110176bf
https://hg.mozilla.org/mozilla-central/rev/c6fb4870676c
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
I realized that the patches break Kakutei-Undo. I'll file a new bug and investigate it Monday.
Depends on: 1174054
Depends on: 1174287
Depends on: 1175382
You need to log in before you can comment on or make changes to this bug.