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 623241 - item edit cursor misplaced
item edit cursor misplaced
Status: RESOLVED FIXED
Product: Gnumeric
Classification: Applications
Component: GUI
git master
Other Linux
: Normal normal
: ---
Assigned To: Jean Bréfort
Jody Goldberg
Depends on:
Blocks:
 
 
Reported: 2010-06-30 20:17 UTC by Andreas J. Guelzow
Modified: 2010-07-13 15:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (7.70 KB, patch)
2010-07-03 15:32 UTC, Jean Bréfort
committed Details | Review
problem screen shot (52.78 KB, image/png)
2010-07-04 17:10 UTC, Andreas J. Guelzow
  Details
Proposed patch for genral text alignment (3.55 KB, patch)
2010-07-11 15:46 UTC, Jean Bréfort
none Details | Review
Updated patch (3.39 KB, patch)
2010-07-12 07:39 UTC, Jean Bréfort
none Details | Review

Description Andreas J. Guelzow 2010-06-30 20:17:04 UTC
new gnumeric
start editing a cell in the edit line. enter unicode characters u05da followed by u05db. In the edit line the cursor is now placed correctly to the left of the last entered character u05db. In the item edit on the other hand it is to the right of character u05da.
Comment 1 Morten Welinder 2010-07-01 00:29:05 UTC
Similarly, we get something wrong when clicking in an array expression.
Comment 2 Morten Welinder 2010-07-01 13:59:53 UTC
Strange.  I don't even the see characters U05da and U05db in the edit line
until I add some ascii characters.
Comment 3 Andreas J. Guelzow 2010-07-01 17:34:49 UTC
are you sure:
<ctrl-shift-u>
0
5
d
a
<space>  (you need to terminate input. This does not add a space character!)
the character should appear on the very right.
Comment 4 Morten Welinder 2010-07-01 19:51:55 UTC
Ah!  I wasn't prepared to look there.
Comment 5 Jean Bréfort 2010-07-02 06:30:02 UTC
Seems that item-edit does not know about rtl mode.
Comment 6 Andreas J. Guelzow 2010-07-02 15:21:40 UTC
I guess we should show the strong cursor (in pango language) since that appears to be shown in the editline (see pango_layout_get_cursor_pos).
Comment 7 Andreas J. Guelzow 2010-07-02 15:36:18 UTC
I was wrong, the edit line shows both cursors:
U+05da U+05db U+05dc 0 1 2
now click to the true right of the 2. The edit line shows the strog cursor in black and the weak cursor in grey.
Comment 8 Andreas J. Guelzow 2010-07-02 15:38:19 UTC
the black and grey may be switched...? It seems that the strong cursor is grey here.
Comment 9 Jean Bréfort 2010-07-02 15:54:20 UTC
I'll have a look at that tomorrow.
Comment 10 Andreas J. Guelzow 2010-07-02 16:13:48 UTC
Jean: thanks

it appears that the edit line always shows just the strong cursor unless the user has clicked in a position where the strong cursor can't be but only the weak cursor. It then sows the weak cursor black and the strong cursor grey.
Comment 11 Jean Bréfort 2010-07-03 14:06:39 UTC
The strong cursor is actually black and the weak cursor grey. This is not very hard to implement, but we probably also should align the text in the item edit just like in the edit line.
Also, selection is not always correctly highlighted. Runs have to be taken into account.
Comment 12 Jean Bréfort 2010-07-03 15:32:24 UTC
Created attachment 165183 [details] [review]
patch

This patch fixes the cursor issue and the selection issue, but the text is not always diplayed correctly and mouse selection is broken when the text is RTL.
Comment 13 Andreas J. Guelzow 2010-07-04 17:05:30 UTC
Comment on attachment 165183 [details] [review]
patch

This is a huge improvement over the current situation. I would suggest you commit this already and then document the remaining problems in this bug report.
Comment 14 Andreas J. Guelzow 2010-07-04 17:10:06 UTC
Created attachment 165229 [details]
problem screen shot

This screenshot shows one remaining problem. 

If we keep the interpretation of "left aligned" (being the physical left) we need to set the correct alignment for the layout.  
If we change the interpretation of "left aligned" to be "logical left , ie. towards the beginning of the text, then we need to move the layout to the correct side of the cell.
Comment 15 Jean Bréfort 2010-07-06 13:25:30 UTC
Comment on attachment 165183 [details] [review]
patch

Actually commited with a few changes because of selection colors issues.
Comment 16 Jean Bréfort 2010-07-06 13:27:14 UTC
Remaining issues in ItemEdit:
- mouse interaction is broken when the text is RTL,
- the item expands to the right instead of the left when the text is RTL and wider than the cell.
Comment 17 Jean Bréfort 2010-07-11 14:47:43 UTC
Mouse interaction is restored and the item now expands to the logic size, at least logic for me.
The screenshot in comment #14 is related to cell rendering, and not to ItemEdit.
Comment 18 Jean Bréfort 2010-07-11 14:55:50 UTC
I'm unable to reproduce what I see in the screenshot. The default position is to the right of the cell. The only issue I see is that when I select left or right align, I obtain the reverse.
Comment 19 Jean Bréfort 2010-07-11 15:46:21 UTC
Created attachment 165683 [details] [review]
Proposed patch for genral text alignment

Please review. Looks like pango alignment logic is reversed when text is RTL. Weird...
Comment 20 Jean Bréfort 2010-07-11 17:15:53 UTC
Forgot to say that previous patch would need analog changes in print-cell.c
Comment 21 Andreas J. Guelzow 2010-07-12 00:07:54 UTC
Review of attachment 165683 [details] [review]:

With this patch I cannot replicate the problem sown in the screen shot any longer, but this patch appears to break the fix for bug #594193, ie. under horizontal fill we see again the unknown-glyph-symbol for the newline character ratehr than the bend arrow symbol.
Comment 22 Jean Bréfort 2010-07-12 07:06:42 UTC
I could fix that but I'm thinking that in filled mode, the text should be right align if rtl. Looks like that should be fixed in cell_calc_layout().
Comment 23 Andreas J. Guelzow 2010-07-12 07:24:45 UTC
I agree that it probably should be right aligned. But I don't see the connection with us not breaking the fix that replaces the unknown glyph char with a known one, especially since your patch is not directly touching that code. So I gather that the code for that fix either is becoming inaccessible or its changes are being reverted by your patch. I did not look at the details for that.
Comment 24 Jean Bréfort 2010-07-12 07:39:00 UTC
Created attachment 165712 [details] [review]
Updated patch

This version does not reintroduce bug #594193.
Comment 25 Jean Bréfort 2010-07-13 15:58:42 UTC
This problem has been fixed in our software repository. The fix will go into the next software release. Thank you for your bug report.
Fixed all alignment bugs I could find.