Closed Bug 1130937 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: Widget: Gtk, defect)

Unspecified
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

(Blocks 2 open bugs)

Details

(Keywords: inputmethod)

Attachments

(3 files)

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
No longer blocks: enable-writing-mode-dev
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
I'll request reviews after trysever is back and the patches are tested on it.
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+
Attachment #8617256 - Flags: review?(m_kato) → review+
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
I realized that the patches break Kakutei-Undo. I'll file a new bug and investigate it Monday.
Depends on: 1174287
You need to log in before you can comment on or make changes to this bug.