Last Comment Bug 1130937 - [Gtk] Need to investigate if IMEs on Linux support vertical writing mode and how do we tells it to IME
: [Gtk] Need to investigate if IMEs on Linux support vertical writing mode and ...
Status: RESOLVED FIXED
: inputmethod
Product: Core
Classification: Components
Component: Widget: Gtk (show other bugs)
: Trunk
: Unspecified Linux
-- normal (vote)
: mozilla41
Assigned To: Masayuki Nakano [:masayuki]
:
:
Mentors:
Depends on: 1174054 1174287 1175382
Blocks: writing-mode 789103
  Show dependency treegraph
 
Reported: 2015-02-08 22:19 PST by Masayuki Nakano [:masayuki]
Modified: 2015-06-16 18:58 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
part.1 nsGtkIMModule should cache selection (18.79 KB, patch)
2015-06-09 03:41 PDT, Masayuki Nakano [:masayuki]
m_kato: review+
Details | Diff | Splinter Review
part.2 nsGtkIMModule should set candidiate window position to bottom left of the target clause in vertical writing mode (11.82 KB, patch)
2015-06-09 03:42 PDT, Masayuki Nakano [:masayuki]
m_kato: review+
Details | Diff | Splinter Review
part.3 nsGtkIMModule should adjust candidate window position when layout is changed (7.02 KB, patch)
2015-06-09 03:42 PDT, Masayuki Nakano [:masayuki]
m_kato: review+
Details | Diff | Splinter Review

Description User image Masayuki Nakano [:masayuki] 2015-02-08 22:19:15 PST
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.
Comment 1 User image Xidorn Quan [:xidorn] (UTC+10) 2015-03-08 18:49:51 PDT
Fctix's author told me that he doesn't know any hint from the system about whether the text should be vertical or not.
Comment 2 User image Masayuki Nakano [:masayuki] 2015-03-09 01:01:32 PDT
(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
Comment 3 User image Makoto Kato [:m_kato] 2015-03-09 01:12:22 PDT
TODO (feature plan)
- We should suggest GTK team (or RedHat's SCIM/ibus team) to add API for vertical layout
Comment 4 User image Masayuki Nakano [:masayuki] 2015-04-30 19:34:40 PDT
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).
Comment 5 User image Masayuki Nakano [:masayuki] 2015-04-30 21:29:58 PDT
Ah, or, should I file a new bug for temporarily fix?
Comment 6 User image Jonathan Kew (:jfkthame) 2015-05-01 04:45:45 PDT
(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.
Comment 7 User image Makoto Kato [:m_kato] 2015-05-10 20:18:33 PDT
add ibus's bug
Comment 8 User image Masayuki Nakano [:masayuki] 2015-06-09 03:41:53 PDT
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.
Comment 9 User image Masayuki Nakano [:masayuki] 2015-06-09 03:42:20 PDT
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
Comment 10 User image Masayuki Nakano [:masayuki] 2015-06-09 03:42:51 PDT
Created attachment 8617257 [details] [diff] [review]
part.3 nsGtkIMModule should adjust candidate window position when layout is changed
Comment 11 User image Masayuki Nakano [:masayuki] 2015-06-10 00:24:30 PDT
Comment on attachment 8617254 [details] [diff] [review]
part.1 nsGtkIMModule should cache selection

First, let's cache selection range and writing mode in nsGtkIMModule.
Comment 12 User image Masayuki Nakano [:masayuki] 2015-06-10 00:29:22 PDT
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.
Comment 13 User image Masayuki Nakano [:masayuki] 2015-06-10 00:31:28 PDT
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.
Comment 14 User image Makoto Kato [:m_kato] 2015-06-11 00:13:24 PDT
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)?
Comment 15 User image Makoto Kato [:m_kato] 2015-06-11 02:07:39 PDT
(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 18 User image Masayuki Nakano [:masayuki] 2015-06-11 19:32:16 PDT
I realized that the patches break Kakutei-Undo. I'll file a new bug and investigate it Monday.

Note You need to log in before you can comment on or make changes to this bug.