Well, there is at least one benefit in it:  Holding ctrl+left would
eventually bring you to the first line of buffer.  Not true about
holding left key alone, or right key alone.

This is functioning as designed, The arrow keys move by *visual*
positions, while control+arrow moves by *logical* words.

Ok, that's a reasonable assumption.  But shouldn't the direction be
switched under a right-to-left locale?  Or something like that? 
That's pretty confusing right now.

Maybe. Currently Ctrl-Left/Right operate as if the keys were labeled


"forward" and "backward"...but maybe RTL people associate the left 
arrow with "backward", or maybe the arrows just too strongly suggest 
physical direction to be used in this way.

We have two options here:

 A) Flip the interpretation of RIGHT/LEFT depending
    on gtk_widget_get_direction (widget), like we
    do for GtkMenuShell.

 B) Try to figure out how to actually navigate visually by
    words, as we currently navigate visually by characters.
    This would probably require Pango API, GtkTextBuffer
    API additions and some real thinking to prevent loops, etc.
   
It might be worth for something to prototype out A), which
should be pretty simple and see how it feels ... that is, is it
satisfactory that when running with a RTL user interface, <- always
moves forward a word logically, -> backwards a word.

Owen, don't we want the flipping in A) to depend on the effective text
direction at the caret position, rather than the overall text
direction of the text view ?

Mathias, doing so based on effective paragraph direction would cause
loops for example when you are going back from an r2l paragraph to l2r
paragraph, and you are stuck in between.  I think a robust logical
movement that only depends on the widget direction (or whatever it is)
is better for now.

*** Bug 162157 has been marked as a duplicate of this bug. ***

*** Bug 302799 has been marked as a duplicate of this bug. ***

*** Bug 394825 has been marked as a duplicate of this bug. ***

*** Bug 408992 has been marked as a duplicate of this bug. ***

*** Bug 509302 has been marked as a duplicate of this bug. ***

*** Bug 543472 has been marked as a duplicate of this bug. ***

*** Bug 560136 has been marked as a duplicate of this bug. ***

what's the status of this bug?

I think this should be fixed. the last dup (filed by me) contains a good (IMHO) explanation why.

Good to see I'm not the only one disturbed by this behavior.

I think my comment 5 from 2004 still captures the status pretty well. It's a a real bug. It's waiting for someone to submit a patch. There's an easy solution that should improve things a bit but would need to be prototyped and tested by a RTL user. And there's a complex solution available for someone who really like puzzles and thinking hard about bidirectional text.

*** Bug 579693 has been marked as a duplicate of this bug. ***

Dear friends,

Perhaps this can help:

(15:42:08) DawnLight: hello. can anyone please point me to the code which deals with the movement of the text cursor in textareas using the keyboard? specifically, it is about http://bugzilla.gnome.org/show_bug.cgi?id=136059
(15:50:35) bz_sleep: DawnLight: you want to start with nsTextControlFrame
15:50:57) bz_sleep: DawnLight: or maybe the plaintext editor directly.....
(15:50:59) bz_sleep ידוע כעת בשם bz
(15:51:28) bz: nsTextInputListener::KeyPress
(15:51:34) bz: and presumably step through from there

Many blessings.

Well I checked version 2.26.3 and there's nothing new... Its still not working, any resolution guys?

Dear friends,

Is anyone willing to solve this for 20$ via PayPal? Patch has to be accepted.

I'll give it a try this week.

This bug has a bounty on it:
http://www.fossfactory.org/project/p197

*** Bug 615927 has been marked as a duplicate of this bug. ***

Created attachment 159141 [details] [review]
Attempt at a fix

Here's a patch that attempts to fix this for the GtkEntry widget.
If it should be filed as a different bug, I'll do so.

(In reply to comment #7)
> Mathias, doing so based on effective paragraph direction would cause
> loops for example when you are going back from an r2l paragraph to l2r
> paragraph, and you are stuck in between.  I think a robust logical
> movement that only depends on the widget direction (or whatever it is)
> is better for now.

How do you suggest deciding the widget direction for GtkTextView?

Hi,
I've tried the patch, and it works well in most cases. It certainly improves the current situation.
Its a little odd when you start moving by-word when the base direction of the widget is different from the text at the cursor position.

I thought of another idea for implementing logical word movement. How about you select the directionality based on the current position of the cursor, but keep it until the user releases the 'Ctrl' key? This would be intuitive in that no matter where you start the movement (in RTL or LTR text), the cursor would move as expected, and in addition, it should prevents loops, as long as the user doesn't release the Ctrl key.

Confirmed in gedit 3.3.5.

This bug was fixed in WebKit changeset 110965 [1]

Their solution is based on ICU, do you think their solution can also solve the current issue in GTK+?

Kind regards,

1: http://trac.webkit.org/changeset/110965

*** Bug 675459 has been marked as a duplicate of this bug. ***

*** Bug 736103 has been marked as a duplicate of this bug. ***

I see other bug reports are being closes as a duplicate of this one, therefore I comment here even though this issue is 11 years old now.

While editing text with gedit on Debian Jessie (8.0) I noticed that I exactly have the same problem as described here. Holding down the control key and pressing arrow keys will move the cursor in the opposite and wrong direction.

I am able to reproduce this bug using gtk3-demo. I am using gedit version 3.14.0.

Created attachment 342499 [details] [review]
Move visually on word movements

I made a patch to address this issue. It applies the visual movement to word movements based on the language. I think this patch improves the current statue of gtktextview. Here are my thoughts:

1. The loop problem already exists when using arrow keys. Simply pressing right or left arrow key in a bidi context will not move the cursor to the beginning or to the end.

2. The loop does not happen when using Ctrl+Up/Down arrow keys (a workaround).

3. With my patch the same loop happens with word movements

4. I noticed Webkit has all the above problems (I tried in Gmail compose box in Chromium on Linux)

I appreciate if anyone tries the patch and share the feedback here. I have used the latest gtk+ git branch (aka Gtk4).

Review of attachment 342499 [details] [review]:

I haven't looked at the code in depth, just some coding style comments.

The commit message should end with the full bug URL, so that we can easily click on it.

In the meantime, the patch needs more testing by real RTL users, to see if the behavior looks good.

::: gtk/gtktextview.c
@@ +6361,3 @@
+  PangoDirection pango_dir;
+  GtkTextLine *line;
+  GtkTextLineDisplay *display = NULL;

Variable declarations must be at the beginning of a block. You can create a new block below the case GTK_MOVEMENT_WORDS, since the variables are needed only there.

@@ +6486,3 @@
+
+      gchar *text = pango_layout_get_text(display->layout);
+      pango_dir = pango_find_base_dir(text, strlen(text));

Wrong coding style. Please add a space before opening parentheses.
See:
https://developer.gnome.org/programming-guidelines/unstable/
https://wiki.gnome.org/Projects/GTK%2B/BestPractices

Created attachment 343512 [details] [review]
Update commit log and code style.

I have applied the latest feedback on code style and commit log.

TL;TR: This is a bump.

I will happily assist anybody interested to try my patch (see my email address in the patch). One does not need to be an RTL user, this patch is all about navigating with Ctrl(+shift) and left/right arrow keys. So testing is easy. Just copy/paste some text for example from http://mehdix.ir/patch.html (Persian which is RTL. Disclaimer my own website) and some English text and try the word navigation.

Moreover, please consider that the RTL users who are skilled enough to come over here and try the patch are rare. This will cause patches like this to remain unaccepted for unlimited time which is bad for the community. However, trying a patch for skilled users is really simple.

I tested Mehdi's patch, using "demos/gtk-demo/gtk4-demo", and it works as expected.

Created attachment 346115 [details] [review]
TextView—Move by words in correct dir for RTL text

It would be great if someone with the right code knowledge and/or RTL usage would review this... all these years later. I don't see anything wrong about it, though I am kinda surprised there isn't an easier way (or someone would've found it). And given that it's so old and so significant for RTL users, if it's confirmed to work by multiple people, surely any fix is a good fix at the moment.

In the meantime, since I don't have that knowledge, I only have everyone's favourite thing... some suggested style nitpicks:
 - const char * for the result of get_text(),
 - avoid a redundant initialisation,
 - cosmetic layout fixes

plus updates to the commit message - mostly to address typos, and explain why/what, not how: those wanting to know the latter will look at the code.

I won't waste your time by making you do these: the updated patch is attached.

I'd be happy to see this go in, though it's a bit wasteful, you'd think we have that information somewhere cached already.  Again, happy to see it go in as is.

That's a good point. Does
    priv->layout->default_style->direction
already contain what we need, rather than wading through Pango to get it?

Humm.  We want the direction of current paragraph.  That doesn't look like is paragraph-dependant.

Created attachment 346119 [details] [review]
TextView—Move by words in correct dir for RTL text

updated again: adds an #include required for 3.22 and passes -1 to pango_find_base_dir() to make it calculate the string length for us

However, there is a problem: for me, this fixes the issue in a plain TextView (e.g. from gtk3-demo), but not in GEdit, which uses a subclass of TextView now.

So unless I have done something silly... that would require further investigation.

The problem in gedit can be subject of another bug. Let's keep this bug focused on gtk. If it is fixed here, it would be easier to fix in downstream.

The reason I asked was to confirm whether you or anyone else who has tested the patch saw the same difference in GEdit. Did you?

Review of attachment 159141 [details] [review]:

This is for a different widget, but seems sensible and does fix the issue there, afaict. Any real RTL users want to confirm?

If so, I would be happy to push this, with the style nitpick that we don't put braces around single-line blocks.

Review of attachment 159141 [details] [review]:

When I apply this patch I get the following error:

gtkentry.c: In function ‘gtk_entry_move_cursor’:
gtkentry.c:5231:20: error: ‘GtkEntry {aka struct _GtkEntry}’ has no member named ‘resolved_dir’
           if (entry->resolved_dir == PANGO_DIRECTION_RTL)

I think this patch is outdated.

I based it on gtk-3-22.

If master doesn't provide that member, we'll have to find another way to fix it, as it currently has the same problem.

Created attachment 346134 [details] [review]
Entry—Move by words in correct dir for RTL text

The problem was that in 2.24, resolved_dir was a member of the Entry struct itself, not EntryPrivate.

This updated version works on 3.22 and master. 2.24 would use the original one.

I applied both patches to gtk-3-22 and both gtkentry and gtktextview work as expected. 

Moreover, I built gedit against these patches. As you stated earlier, they don't fix gedit, because it uses gtksourceview which is an independent project. I looked at its code and it looks like a modified version of gtktextview, but still very similar. I will create a similar patch for them.

Thanks for checking.

What I don't get is that GtkSourceView is a subclass of GtkTextView and does not appear to override any handling for arrow keys. But clearly something is going on.

I guess the same patch should added to the move_cursor_words:
https://git.gnome.org/browse/gtksourceview/tree/gtksourceview/gtksourceview.c#n2084

or gtk_source_view_move_cursor function:
https://git.gnome.org/browse/gtksourceview/tree/gtksourceview/gtksourceview.c#n2119

As far as I understood these two functions should be modified in order to make visual word moves work. I can't tell that the code is pushing the movement to its parent (gtktextview) or simply is using copy pasted code from it.

Created attachment 346141 [details]
video of 'ghost' cursor in Entry at opposite side of word

I just realised that when moving between words in a GtkEntry containing RTL text - with or without the patch - sometimes a 'ghost' of the cursor is left at the other side of the word than we just moved to.

Not directly related, but seemed worth mentioning

(In reply to Mehdi Sadeghi from comment #51)
> I guess the same patch should added to the move_cursor_words:
> https://git.gnome.org/browse/gtksourceview/tree/gtksourceview/gtksourceview.
> c#n2084
> 
> or gtk_source_view_move_cursor function:
> https://git.gnome.org/browse/gtksourceview/tree/gtksourceview/gtksourceview.
> c#n2119
> 
> As far as I understood these two functions should be modified in order to
> make visual word moves work. I can't tell that the code is pushing the
> movement to its parent (gtktextview) or simply is using copy pasted code
> from it.

Good spot. The class_init() function overwrited the class's vtable:
    textview_class->move_cursor = gtk_source_view_move_cursor,
so that gets called first, and for MOVEMENT_WORDS, we return before the default chain-up to the TextView implementation, so SourceView totally takes over responsibility for the cursor movement.

As for which site you should patch - it depends on whether you find the count also needs inverted for DISPLAY_LINE_ENDS and PARAGRAPH_ENDS cases. If these refer to shortcuts like Ctrl+up/down, then a quick test indicates to me that those already work OK in RTL text (as do Home/End, which may also be relevant).

There is at least one other case that we need to account for: at least for TextView, we also need to invert the count if we have an existing selection and are performing a movement to either of its bounds.

Otherwise, if you select some text and then do Ctrl+left/right to move to the nearest word end from either side - you get taken to the other side.

Do you see this too?

Created attachment 346154 [details] [review]
Entry—Fix inverted movements by arrow keys in RTL

This attempts to handle the case of cancelling a selection too, so that we move to the right side of it.

Detailed testing and feedback would be much appreciated. It took me a while to find the right circumstances in which to invert the count, and I might still be breaking some edge case that I've not thought of yet.

Created attachment 346155 [details] [review]
TextView—Fix inverted movements by arrows in RTL

Same as for Entry - tries to also cover the case of moving to end of/cancelling a pre-existing selection. And just the same, detailed testing and feedback would be greatly appreciated.

Created attachment 346156 [details] [review]
Entry—Fix inverted movements by arrow keys in RTL

Unlike the previous version, don't invert on a LOGICAL movement case. I didn't find anything that broke due to that yet, but it doesn't seem right, so don't do it.

(In reply to Daniel Boles from comment #52)
> Created attachment 346141 [details]
> video of 'ghost' cursor in Entry at opposite side of word
> 
> I just realised that when moving between words in a GtkEntry containing RTL
> text - with or without the patch - sometimes a 'ghost' of the cursor is left
> at the other side of the word than we just moved to.
> 
> Not directly related, but seemed worth mentioning

I think those ghost cursors are expected. Your sample text contains Arabic numerals wich are written left-to-right. When cursor arrives at a numeral, the ghost cursor shows the effective place that any entered number will be displayed.

(In reply to Daniel Boles from comment #54)
> There is at least one other case that we need to account for: at least for
> TextView, we also need to invert the count if we have an existing selection
> and are performing a movement to either of its bounds.
> 
> Otherwise, if you select some text and then do Ctrl+left/right to move to
> the nearest word end from either side - you get taken to the other side.
> 
> Do you see this too?

Yes I see. I just tried with 3.22 branch. With the older patch, the problem that you described happened. However, applying the latest patch with your modification solves the problem.

OK, after 13 years, I'm going to take a gamble that these patches are good enough for the time being and land them, finally. If any corner cases show up later, we can file shiny new bugs.

Ori and Mehdi: Thanks for the patches for Entry and TextView!

Mehdi: Please file another bug for this in GtkSourceView, in its own category. Thanks also for the testing and info you've provided.

> OK, after 13 years...

Thanks!  Back then I didn't even have commit access to GNOME repos!

Thanks to Mehdi & Daniel to finally improve the state of Ctrl-navigation! :)

(and don't forget Ori! :D)

Oh yeah, sorry I overlooked that. :)
And sorry for not helping even in testing; I could only go with pre-built binaries for my own system (F25 :P).

I just opened bug #778928 for gtksourceview to follow up with the rest of the problem and eventually fix it.

Thanks to all for your help in closing this old bug.

(In reply to Daniel Boles from comment #56)
> Created attachment 346155 [details] [review] [review]
> TextView—Fix inverted movements by arrows in RTL
> 
> Same as for Entry - tries to also cover the case of moving to end
> of/cancelling a pre-existing selection. And just the same, detailed testing
> and feedback would be greatly appreciated.

I have some comments for the committed patch:
https://git.gnome.org/browse/gtk+/commit/?id=0128b

It lacks a corresponding gtk_text_layout_free_line_display() to not leak the line_display, also needs to include gtktextiterprivate.h to avoid a gcc warning for _gtk_text_iter_get_text_line(). But these are minor things, my main concern is on using the heavy function gtk_text_layout_get_line_display() just to get the bidi direction (same concern behdad said on comment 38).

So I've made a new patch improving this, extracting the find bidi direction logic from gtk_text_layout_free_line_display() to its own function so it can be used by the textview cursor movement code.

In my tests the patch performs as well as the committed one.

Created attachment 346425 [details] [review]
gtktextview.c: improve fix for cursor movement in rtl

(In reply to Nelson Benitez from comment #66)
> (In reply to Daniel Boles from comment #56)
> So I've made a new patch improving this, extracting the find bidi direction
> logic from gtk_text_layout_free_line_display() to its own function so it can
> be used by the textview cursor movement code.
> 
> In my tests the patch performs as well as the committed one.

Would it be possible to expose this functionality publicly in order to fix bug #778928 in gtksourceview and other widgets that might need to improve their RTL behaviour?

(In reply to Nelson Benitez from comment #66)
> It lacks a corresponding gtk_text_layout_free_line_display() to not leak the
> line_display,

I'll check and patch this later (unless I go straight to your patch below)


> also needs to include gtktextiterprivate.h to avoid a gcc
> warning for _gtk_text_iter_get_text_line().

You are linking to the version of the commit in master, where that header was already included prior to my patch.

For the gtk-3-22 branch, you'll see I added the include:
https://git.gnome.org/browse/gtk+/commit/?h=gtk-3-22&id=3e5d5f8899b6fa389ec781ef5f6e40d7d311cfd1

(Also, I updated the patch slightly after the version attached here, mostly to declare variables in fewer lines.)


> But these are minor things, my
> main concern is on using the heavy function
> gtk_text_layout_get_line_display() just to get the bidi direction (same
> concern behdad said on comment 38).

Wow, yeah. I hadn't actually looked into that function. Not only did I assume it was returning a reference to an existing object - I had no idea it was doing so much heavy lifting every time.

Still, I agreed with Behdad that for now, it was good enough until someone found a superior way - so thanks for doing so!


> So I've made a new patch improving this, extracting the find bidi direction
> logic from gtk_text_layout_free_line_display() to its own function so it can
> be used by the textview cursor movement code.
> 
> In my tests the patch performs as well as the committed one.

Thanks - this looks great at a first glance. I'll check it later and commit if all is well.

Comment on attachment 346155 [details] [review]
TextView—Fix inverted movements by arrows in RTL

committed with slight amendments

(In reply to Mehdi Sadeghi from comment #68)
> Would it be possible to expose this functionality publicly in order to fix
> bug #778928 in gtksourceview and other widgets that might need to improve
> their RTL behaviour?

This does seem like a method that can be broadly useful.

If committed as is, it would already be usable by other widgets' implementations: we would just not be promising API stability for public use (as indicated by the leading underscore* and lack of documentation). I don't see a problem there.

* which makes the C Standard very sad :(

Review of attachment 346425 [details] [review]:

::: gtk/gtktextlayout.h
@@ +473,3 @@
 void gtk_text_layout_spew (GtkTextLayout *layout);
 
+GDK_AVAILABLE_IN_ALL

not required as this is an internal/private method, but if it were public, it would need to be IN_3_22

Review of attachment 346425 [details] [review]:

::: gtk/gtktextlayout.h
@@ +474,3 @@
 
+GDK_AVAILABLE_IN_ALL
+PangoDirection _gtk_text_layout_find_line_base_dir (GtkTextLayout *layout,

You have to choose: either it's public, and needs the annotation; or it's private, in which case the annotation goes away.

In either cases, the leading underscore has to be removed.

As a general consideration, we do not add any new API in GTK+ 3.22. In this case, we may make an exception, given the fact that GtkSourceView may use this function, but it will need further discussion.

(In reply to Emmanuele Bassi (:ebassi) from comment #73)
> In either cases, the leading underscore has to be removed.

What is the policy for using or not using the leading underscore? There are a lot of those floating around (notably including every single method of GtkTreeMenu)

(In reply to Daniel Boles from comment #74)
> (In reply to Emmanuele Bassi (:ebassi) from comment #73)
> > In either cases, the leading underscore has to be removed.
> 
> What is the policy for using or not using the leading underscore? There are
> a lot of those floating around (notably including every single method of
> GtkTreeMenu)

The underscore was used back when all functions where public by default and we used a regular expression to generate the symbols table at build time. Now all symbols are private by default, unless annotated with a GDK_AVAILABLE_* macro.

The underscore is in the process of being phased out; just like we don't do whitespace fixes to avoid breaking `git blame` and logging, we don't modify the name of existing functions unless we're also changing their implementation. If you are adding a new internal symbol, then you should not use a leading underscore (or annotate it with G_GNUC_INTERNAL).

I just asked on Gtk+ IRC channel how I can get GtkTextLine object in GtkSourceView (in order to call the new function):
14:18 < mehdi> Hi there. Is it possible to get a GtkTextLine object in Gtksourceview for the current line? 
14:20 < mclasen___> thats an internal data structure
14:21 <@ebassi> The API is, sadly, semi-public
14:23 <@ebassi> mehdi: gtk_text_layout_get_lines() will return a list of GtkTextLine
14:24 <@ebassi> mehdi: But, again, it's all semi-private
14:27 < mclasen___> in gtk4, its all gone

I wonder whether GtkTextLine as input type is good enough for a public function.

(In reply to Mehdi Sadeghi from comment #76)
> 14:27 < mclasen___> in gtk4, its all gone

What is? Not gtk_text_layout_get_lines():

https://git.gnome.org/browse/gtk+/tree/gtk/gtktextlayout.c?h=master#n708

In master, gtktextlayout.h is not an installed header any more, so you cannot access GtkTextLine and GtkTextLayout.

(In reply to Mehdi Sadeghi from comment #68)
> (In reply to Nelson Benitez from comment #66)
> > (In reply to Daniel Boles from comment #56)
> > So I've made a new patch improving this, extracting the find bidi direction
> > logic from gtk_text_layout_free_line_display() to its own function so it can
> > be used by the textview cursor movement code.
> > 
> > In my tests the patch performs as well as the committed one.
> 
> Would it be possible to expose this functionality publicly in order to fix
> bug #778928 in gtksourceview and other widgets that might need to improve
> their RTL behaviour?

Please, file a new bug for this.

Created attachment 346460 [details] [review]
New patch that adds GtkTextView public API to be used by eg. GtkSourceView

Ok, after reading comments I'm attaching new version of my last patch, this time adding the function as public API so it can be used by external code to support rtl, which is GtkSourceView at this moment, for which I'll attach its corresponding patch too.

I've used GDK_AVAILABLE_IN_3_22 in the patch, but should be changed to relevant version this will go in.

Created attachment 346461 [details] [review]
Patch to fix GtkSourceView handling of RTL 'CTRL + cursor' movement

I've tested this in gedit an works as well as the other patch in plain textview.

I think Emmanuele wanted a new bug created for ongoing discussion on the proposed API. At 80 comments, this one has about run its course in terms of being readable by a human!

(In reply to Emmanuele Bassi (:ebassi) from comment #79)
> Please, file a new bug for this.

I opened bug #778928 and I put links to Nelson's patches. I'll update GtkSourceView bug page too.

BTW, I applied Nelson's patches. They fix the problem perfectly. GtkTextView behaves as before and gedit as expected. Can't wait to see this change in the mainline.

Sorry, wrong bug number. The correct one is bug #779081

Comment on attachment 346460 [details] [review]
New patch that adds GtkTextView public API to be used by eg. GtkSourceView

Please repost for review at Bug 779081

Comment on attachment 346461 [details] [review]
Patch to fix GtkSourceView handling of RTL 'CTRL + cursor' movement

Please repost for review at Bug 778928 (in the event that Bug 779081 succeeds)

Created attachment 346499 [details] [review]
TextView: Plug a memory leak

Thanks to Nelson Benitez for pointing this out.

Created attachment 346500 [details] [review]
TextView—Avoid pointless Pango in iter_line_is_rtl

Get the direction that was already worked out and stored in the
TextLineDisplay, rather than making Pango figure it out again.

Review of attachment 346499 [details] [review]:

::: gtk/gtktextview.c
@@ +6425,3 @@
   PangoDirection pango_dir = pango_find_base_dir (text, -1);
 
+  g_object_unref (display);

GtkTextLineDisplay is not a GObject.

g_slice_free() creates crashes. Whuh?

Created attachment 346510 [details] [review]
TextView: Plug a memory leak

Thanks to Nelson Benitez for pointing this out.

Created attachment 346511 [details] [review]
TextView—Avoid pointless Pango in iter_line_is_rtl

Get the direction that was already worked out and stored in the
TextLineDisplay, rather than making Pango figure it out again.

*** Bug 790341 has been marked as a duplicate of this bug. ***