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 588089 - Incorrect signal parameters when deleting composed character
Incorrect signal parameters when deleting composed character
Status: RESOLVED NOTABUG
Product: gtk+
Classification: Platform
Component: Widget: GtkTextView
2.14.x
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2009-07-08 15:46 UTC by Walter Leibbrandt
Modified: 2009-07-16 23:00 UTC
See Also:
GNOME target: ---
GNOME version: 2.25/2.26


Attachments
Proposed fix (1.69 KB, patch)
2009-07-16 14:47 UTC, Walter Leibbrandt
rejected Details | Review

Description Walter Leibbrandt 2009-07-08 15:46:06 UTC
Please describe the problem:
When using backspace to delete a composing character in a gtk.TextView, the associated TextBuffer fires the "delete-range" event with incorrect parameters. The start- and end iterators have an offset difference of 2, even though only one character is deleted.

Similar behavior is observed when there are more than one composing character used. For example ប្តូ (Khmer OS fonts needed - ttf-khmeros package in Ubuntu), which contains 4 Unicode codepoints, the last 3 of which are composing characters.

I realize that this is quite possibly a gtk+ bug and not pygtk, but I can't confirm.

Steps to reproduce:
1. Set the text in a TextView toنَ 
2. Press backspace.

Actual results:
The result in the TextView is ن, but the iterator arguments to the "delete-range" signal have offsets 0 and 2.


Expected results:
The start- and end iterators should have offset values of 1 and 2, respectively.

Does this happen every time?
Yes

Other information:
Comment 1 Gian Mario Tagliaretti 2009-07-16 00:36:10 UTC
I don't think that PyGTK is doing anything special with this signal, most likely something that should be investigated in GTK+ itself.

Reassigning to GTK+ is I'm wrong please reassign back.
Comment 2 Walter Leibbrandt 2009-07-16 09:16:27 UTC
(In reply to comment #1)
> I don't think that PyGTK is doing anything special with this signal, most
> likely something that should be investigated in GTK+ itself.
> 
> Reassigning to GTK+ is I'm wrong please reassign back.
> 

You are correct. I've tracked the bug to the implementation of gtk_text_buffer_backspace() in gtktextbuffer.c.

It deletes the range between the cursor and the result of gtk_text_iter_backward_cursor_position(), which (apparently) does not take combined characters into account. After this deletion the character's PangoLogAttr.backspace_deletes_character is checked. If that attribute is true (as is the case with (most) combined characters) the deleted text is inserted again, but with the length of the combining character subtracted from the length parameter to gtk_text_buffer_insert_interactive().

This all means that this bug is actually more of a symptom of the real bug.

I'm working on a patch, although my C skills are quite rusty.
Comment 3 Walter Leibbrandt 2009-07-16 14:47:48 UTC
Created attachment 138534 [details] [review]
Proposed fix

Changes to gtk_text_buffer_backspace() to test the "backspace_deletes_character" attribute before actually deleting anything.

This patch was not tested in any way, seeing as I don't have the resources to setup a build environment.
Comment 4 Owen Taylor 2009-07-16 15:47:54 UTC
What GTK+ does currently is:

 - deletes the old text of the grapheme
 - inserts the new text for the grapheme

Patch is definitely wrong. As a hypothetical example, imagine thatbackspace_deletes_character was true for 'á' - then backspace after á would replace the á with an a.

It would be possible to do:

 - Compute the before and after text
 - See if the after text is a prefix of the before text
 - If so, optimize the ::delete-range ::insert-text to a single ::delete-range

I don't really see the point though - apps need to handle the concept that a single user action can be split ito multiple independent inserts and deletes. That's why there is are the ::begin-user-action and ::end-user-action signals.

(Another example where this hapens - pasting while there is a selection.)
Comment 5 Ian Galpin 2009-07-16 23:00:32 UTC
(In reply to comment #4)
> What GTK+ does currently is:
> 
>  - deletes the old text of the grapheme
>  - inserts the new text for the grapheme
> 
> Patch is definitely wrong. As a hypothetical example, imagine
> thatbackspace_deletes_character was true for 'á' - then backspace after á
> would replace the á with an a.
Correct me if I'm wrong, if that were to happen, would that not be a problem with backspace_deletes_character being true for a character when it should not be?  If that is the case, this patch would expose bugs that lie else where, which is always a good thing :)

> 
> It would be possible to do:
> 
>  - Compute the before and after text
>  - See if the after text is a prefix of the before text
>  - If so, optimize the ::delete-range ::insert-text to a single ::delete-range
> 
> I don't really see the point though - apps need to handle the concept that a
> single user action can be split ito multiple independent inserts and deletes.
> That's why there is are the ::begin-user-action and ::end-user-action signals.
> 
> (Another example where this hapens - pasting while there is a selection.)
> 

How does this patch affect programs that handle multiple insert/delete events correctly?  The patch seems to be "Doing The Right Thing"(TM) by not creating an insert event if it is not needed.