After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 756141 - im cursor location not updated on viewport scroll
im cursor location not updated on viewport scroll
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
git master
Other Linux
: Normal minor
: ---
Assigned To: VTE Maintainers
VTE Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-10-06 20:12 UTC by Egmont Koblinger
Modified: 2016-03-15 04:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch for vte.cc (2.01 KB, patch)
2016-01-22 09:01 UTC, Takao Fujiwara
none Details | Review
Patch for vte.cc (1016 bytes, patch)
2016-02-08 06:57 UTC, Takao Fujiwara
none Details | Review

Description Egmont Koblinger 2015-10-06 20:12:10 UTC
Produce more than a screenful of output.

Switch to "Japanese (Anthy) (IBus)" keyboard layout.

Scroll back with the scrollbar a bit.

Press a letter or two, followed by two spaces, so that a popup window containing several kanjis appear.

Notice that this window is incorrectly positioned vertically, as if you didn't scroll back the viewport.

gtk_im_context_set_cursor_location() is now only called from vte_terminal_process_incoming(), should also be called when scroll_delta changes.
Comment 1 Egmont Koblinger 2015-10-06 20:17:59 UTC
Also, the horizontal position takes neither preedit_cursor, nor the amount by which it's shifted to the left (see bug 755668) into account.
Comment 2 Sebastian 2016-01-19 09:10:47 UTC
I believe that this bug also occurs when the cursor is on the last line of the terminal and one presses enter. Pressing enter causes the terminal content to scroll down one line but the coordinates that ibus receives are now outside the terminal window.
Comment 3 Takao Fujiwara 2016-01-22 09:01:52 UTC
Created attachment 319542 [details] [review]
Patch for vte.cc

The patch calls gtk_im_context_set_cursor_location() if the preedit is changed.
Comment 4 Christian Persch 2016-01-31 17:06:09 UTC
I don't think updating the IM's cursor position during ::draw is the right place. I've committed a patch that factors out the update, and call that when the preddit string changes. Still need to add a call when the scroll_delta changes, I think.
Comment 5 Takao Fujiwara 2016-02-03 09:30:25 UTC
The integrated patch does not take the preedit length.
My patch is:
rect.x = m_screen->cursor.col * m_char_width + m_padding.left + get_preedit_width(false) * m_char_width;

To reproduce, launch ibus-setup and select "Hide automatically" in "Show property panel" and then you could see the difference between gtk and vte.
Comment 6 Takao Fujiwara 2016-02-03 09:36:47 UTC
--- a/src/vte.cc
+++ b/src/vte.cc
@@ -4647,7 +4647,8 @@ VteTerminalPrivate::im_update_cursor()
                 return;
 
         cairo_rectangle_int_t rect;
-        rect.x = m_screen->cursor.col * m_char_width + m_padding.left;
+        rect.x = m_screen->cursor.col * m_char_width + m_padding.left +
+                 get_preedit_width(false) + m_char_width;
         rect.width = m_char_width; // FIXMEchpe: if columns > 1 ?
         rect.y = row_to_pixel(m_screen->cursor.row) + m_padding.top;
         rect.height = m_char_height;

Actually current patch fixes the scroll_delta change too, I think.
Comment 7 Takao Fujiwara 2016-02-08 06:57:56 UTC
Created attachment 320603 [details] [review]
Patch for vte.cc

Updated the patch.
Comment 8 Christian Persch 2016-03-14 17:13:04 UTC
> get_preedit_width(false) + m_char_width

That should have been '*' not '+', right? Committed with that change.
Comment 9 Takao Fujiwara 2016-03-15 04:32:21 UTC
(In reply to Christian Persch from comment #8)
> > get_preedit_width(false) + m_char_width
> 
> That should have been '*' not '+', right? Committed with that change.

You're right. Thank you for the fix.