View Issue Details

IDProjectCategoryView StatusLast Update
0019220LazarusWidgetsetpublic2012-03-15 17:44
ReporterBernd Kreuss Assigned ToZeljan Rikalo  
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Product Version0.9.31 (SVN) 
Summary0019220: TEdit inserts at wrong position when parts of the text are selected
DescriptionSteps to reproduce:
1: put a TEdit onto a form and compile and run the application
(or alternatively just open one of the IDE's dialogs which contains a
TEdit like for example refactor -> rename)

2. write some text into it and then select a few characters in the
middle of the word. For example I will write the text
"TFooBar" into the TEdit and then select the three letters "Foo".
Then press any (printable) key.

3: I expect it to look like this (I have pressed the letter "X" and in the following the | marks the position of the caret), this is what I expect:
TX|Bar

4. And this is what happens:
TBX|ar

It removes the "Foo" as expected but then jumps over the B and inserts after it.
Additional InformationIt is a regression but I cannot say which version exactly broke it since I did not use it and not update for a few months, I would guess there is a range of a few hundred revision numbers where it could have happened.

I have not yet checked whether this has also somehow slipped into the 0.9.30 branch but I cannot remember having seen this behavior in the trunk during the time when the release branch was stabilized.

I have initially marked it as "major" because - although some people might never notice it at all - the TEdit with this behavior might be perceived as nearly unusable by users with a certain style of editing text with the keyboard.
TagsNo tags attached.
Fixed in Revision30461,35986
LazTarget-
WidgetsetGTK 2
Attached Files

Activities

Bernd Kreuss

2011-04-24 11:17

reporter   ~0047732

My GTK2 is the one that came with Ubuntu Hardy:

pkg-config --modversion gtk+-2.0
2.12.9

Linux, i386

Bernd Kreuss

2011-04-24 12:44

reporter   ~0047734

The bug is also in the official 0.9.30 i386 .deb files from sourceforge :-(

Juha Manninen

2011-04-24 12:59

developer   ~0047735

Bernd, you could get the exact revision by "bisecting". I can't do it now myself because I can't reproduce this bug.
GTK 2.24.3 here.

Bart Broersma

2011-04-24 14:04

developer   ~0047737

Last edited: 2011-04-24 14:13

Using gtk2-2.8.3-4 and Lazarus 0.9.31 r29157
(See attached sample program)

Starting with TFooBar in TEdit and having Foo selected, this happens:
(The | indicates the position of the caret after pressing the key)

Press Backspace:
Edit1KeyPress: Text = "TFooBar" SelLength = 3 SelStart = 1, SelText = "Foo"
Edit1Change: Text = "TBar" SelLength = 0 SelStart = 2, SelText = ""
//SelStart should be 1 here
TB|ar

Press Delete
Edit1Change: Text = "TBar" SelLength = 0 SelStart = 1, SelText = ""
T|Bar /this is correct

Press x
Edit1KeyPress: Text = "TFooBar" SelLength = 3 SelStart = 1, SelText = "Foo"
Edit1Change: Text = "TBar" SelLength = 0 SelStart = 2, SelText = ""
Edit1Change: Text = "TBxar" SelLength = 0 SelStart = 3, SelText = ""
//SelStart should be 1 here
TBx|ar // incorrect text and incorrect caret placement

2011-04-24 14:12

 

tedit_selstart.tgz (3,580 bytes)

Bart Broersma

2011-04-24 15:00

developer   ~0047738

The issue was introduced in r24030:
In that revsion you will end up with TBarx|

This was partially fixed in r24043, since that you'll end up with TBx|ar

2011-04-24 15:26

 

gtk2callback.inc.diff (525 bytes)   
Index: gtk2callback.inc
===================================================================
--- gtk2callback.inc	(revision 29157)
+++ gtk2callback.inc	(working copy)
@@ -436,7 +436,7 @@
         // if we change selstart in OnChange event new cursor pos need to
         // be postponed in TGtk2WSCustomEdit.SetSelStart
         NeedCursorCheck := True;
-        gtk_editable_set_position(PGtkEditable(Widget), GStart + 1);
+        gtk_editable_set_position(PGtkEditable(Widget), GStart {+ 1});
       end;
     end;
   end;
gtk2callback.inc.diff (525 bytes)   

Bart Broersma

2011-04-24 15:27

developer   ~0047739

Uploaded a possible fix for this.

I did not check wether the same issue also exists in GTK(1).

Bernd Kreuss

2011-04-24 15:58

reporter   ~0047740

@bart you were faster than me. I just completed dozens of makes in a binary search to narrow it down and found the same: 24030 breaks it badly, the cursor jumps completely to the end and 24043 partially fixed it.


bernd@t40:~/lazsvn/lazarus/trunk\ $ svn log -r24030
------------------------------------------------------------------------
r24030 | zeljko | 2010-03-16 10:14:54 +0100 (Di, 16. Mär 2010) | 2 Zeilen

Gtk2: provoke gtkEditable to update GtkEntry cursorpos property inside changed event. fixes 0007243


------------------------------------------------------------------------
bernd@t40:~/lazsvn/lazarus/trunk\ $ svn log -r24043
------------------------------------------------------------------------
r24043 | zeljko | 2010-03-16 16:34:28 +0100 (Di, 16. Mär 2010) | 2 Zeilen

Gtk2: rework cursor_pos update for GtkEntry.Now it works correct and completely fixes 0007243

------------------------------------------------------------------------

Bernd Kreuss

2011-04-24 16:23

reporter   ~0047741

Last edited: 2011-04-24 16:25

the patch from Bart (gtk2callback.inc.diff) fixes the problem for me but I don't know the implications for the ones who did not have the bug previously.

Juha Manninen

2011-04-25 08:20

developer   ~0047751

Bart's patch works with new GTK2 versions, too.

gtk_editable_set_position seems to be a No_Operation call in new versions.
I tested also:
  gtk_editable_set_position(PGtkEditable(Widget), GStart+5);
and it made no difference.

Please test.

Bernd Kreuss

2011-04-25 08:58

reporter   ~0047752

Thank you for the extremely fast reaction and solution, even during the holidays and prolonged weekend. r30461 does fix it for me (old gtk2-2.12.9), as far as I am concerned this bug can be closed.

I vote for this to be also merged into to the 0.9.30-fixes branch. Is there a separate list somewhere of revisions that are nominated for being merged into the fixes branch or should I file a separate bug against 0.9.30.1?

Bart Broersma

2011-04-25 10:26

developer   ~0047754

@Bernd: "even during the holidays and prolonged weekend", it's because of that I have the time ...

Juha Manninen

2011-04-25 13:50

developer   ~0047755

> I vote for this to be also merged into to the 0.9.30-fixes branch.

No need for that. There the code is identical with the new fixed code. I wonder why it was changed in the first place.

Bernd Kreuss

2011-04-25 14:38

reporter   ~0047757

> No need for that. There the code is identical with the new fixed code. I wonder why it was changed in the first place.

Hmm... I deduced this from installing and trying the original 0.0.30 release .debs yesterday. But I did not try the fixes nor did I look into the code of 0.9.30.fixes or the svn log. Maybe it has been changed there independently.

However. Its working now :-)

Juha Manninen

2011-04-25 17:35

developer   ~0047763

> Hmm... I deduced this from installing and trying the original 0.0.30 release .debs yesterday.
> But I did not try the fixes nor did I look into the code of 0.9.30.fixes or the svn log.
> Maybe it has been changed there independently.

Ok, I managed somehow to look at a wrong file. 0.9.30-fixes branch did have the same problem and now the patch is applied there, too (r30471).

Bart Broersma

2011-04-26 18:08

developer   ~0047805

I'm sorry but my fix is wrong, it causes yet another bug: 0019239

Can someone plese revert this patch (in both trunk and 0.9.30.1)?

Jesus Reyes

2011-04-26 18:47

developer   ~0047810

reverted in r30481

Juha Manninen

2011-04-26 19:27

developer   ~0047815

Reverted also in fixes branch.
Good lesson for me. Changes should never be applied to fixes at once but should be tested for some time in trunk.

Someone else needs to take over this case. I don't know enough of GTK2 bindings.
Zeljan, Paul ?

Bart Broersma

2011-04-27 22:00

developer   ~0047855

function gtkchanged_editbox( widget: PGtkWidget; data: gPointer) : GBoolean; cdecl;
var
  Mess : TLMessage;
  GStart, GEnd: gint;
  Info: PWidgetInfo;
  EntryText: PgChar;
  NeedCursorCheck: Boolean;
begin
  Result := CallBackDefaultReturn;

  if LockOnChange(PgtkObject(Widget),0)>0 then exit;
  {$IFDEF EventTrace}
  EventTrace('changed_editbox', data);
  {$ENDIF}
  NeedCursorCheck := False;
  if GTK_IS_ENTRY(Widget) then
  begin
    {cheat GtkEditable to update cursor pos in gtkEntry. issue 0007243}
    gtk_editable_get_selection_bounds(PGtkEditable(Widget), @GStart, @GEnd);
    EntryText := gtk_entry_get_text(PGtkEntry(Widget));
    if (GStart = GEnd) and
      (UTF8Length(EntryText) >= PGtkEntry(Widget)^.text_length) then
    begin
      Info := GetWidgetInfo(Widget, False);
      {do not update position if backspace or delete pressed}
      if wwiInvalidEvent in Info^.Flags then
        exclude(Info^.Flags, wwiInvalidEvent)
      else
      begin
        // if we change selstart in OnChange event new cursor pos need to
        // be postponed in TGtk2WSCustomEdit.SetSelStart
        NeedCursorCheck := True;
        gtk_editable_set_position(PGtkEditable(Widget), GStart + 1);
/////////////^^^^ this line is incorrect if you type when the edit has text selected
      end;
    end;
  end;

  if NeedCursorCheck then
    LockOnChange(PgtkObject(Widget), +1);
  FillByte(Mess,SizeOf(Mess),0);
  Mess.Msg := CM_TEXTCHANGED;
  DeliverMessage(Data, Mess);
  if NeedCursorCheck then
    LockOnChange(PgtkObject(Widget), -1);
end;

If you type over a selection, then this function is called twice, first when the selection is deleted, second when the new text is inserted where the selection was. In both instances the value of GStart is incremented, but that should only happen once.
In gtkchanged_editbox() there is no way of telling which situation occurs (GEnd in this case still has the same value as GStart).

Again, if we start with "TFooBar" as the text, with "foo" selected and then type "x", this will happen in gtkchanged_editbox():

EntryText = TBar GStart = 1 GEnd = 1
(wwiInvalidEvent in Info^.Flags) = False
EntryText = TBxar GStart = 2 GEnd = 2
(wwiInvalidEvent in Info^.Flags) = False

In the first step (when the selection gets deleted) is it possible to set the wwiInvalidEvent flag (thus avoiding changing the cursorposition)?

Bernd Kreuss

2011-04-28 08:04

reporter   ~0047861

maybe there are even more GTK signals available that could be bound to callback methods as it has been done with backspace and delete already in r24043 (the one which tried to fix the original fix, the one that introduced wwiInvalidEvent).

Or maybe there even exists an entirely different or more "natural" way to fix the original issue 0007243, something that does not mess so deeply with internal GTK implementation details and behaviors.

Bart Broersma

2011-04-28 14:23

developer   ~0047876

Changed status to confirmed, the status was "assigned" but to nobody, which seemed odd to me (and it might erroneously suggest that someone is working on it).

Bart Broersma

2011-04-28 20:20

developer   ~0047890

Some observations:
- If you paste over the selected text, then this bug will not happen.
- The deleting of the selected text by typing over it, does not trigger the "delete-from-cursor" or "backspace" signal.
Maybe take a look at the "delete-text" signal from GtkEditable?
(I have no idea how to test this...)

Reading the docs on developer.gnome.org, it states this about the "changed" signal from GtkEditable:

The ::changed signal is emitted at the end of a single user-visible operation on the contents of the GtkEditable.
E.g., a paste operation that replaces the contents of the selection will cause only one signal emission (even though it is implemented by first deleting the selection, then inserting the new content, and may cause multiple ::notify::text signals to be emitted).

This clearly seems not the case (see note 0047855)?

Zeljan Rikalo

2012-03-14 11:29

developer   ~0057626

Ok, lets see what's up here.

1. I've tested with trunk + gtk2 2.24.8 and cannot reproduce (0.9.31 r35965)
2. What gtk lib versions have problems ? Bart ? 2.8 again ?

@Bernd do you have problems with this anymore ?

Bart Broersma

2012-03-14 12:40

developer   ~0057635

@Zeljan: problem remains.
Start with abc123def in TEdit, select 123, type x ==> result = abcdx|ef (| is cursorposition)
I applied your patch for issue 18697 (gtk2floatspinedit_rework.diff) and it makes no difference.

Lazarus r35947
Fpc 2.6.0
gtk2-2.8.3-4
Suse Linux 10.0 (Kernel 2.6.13-15)

Note that this behaviour actually is a regression. I have a compiled Lazarus program with a TEdit, compiled at 2009-10-12 which does not show this bug.
Unfortunately I have no idea which Lazarus revision it was built with (probably 0.9.29).

Zeljan Rikalo

2012-03-14 15:48

developer   ~0057645

Yes, with 2.8 that bug exists. Problem is that I want to know which gtk2 versions are affected .... anyone with > 2.8 and < 2.24 ?

Zeljan Rikalo

2012-03-14 17:32

developer   ~0057648

Please test and close if ok.

Zeljan Rikalo

2012-03-14 17:36

developer   ~0057649

Also checked if 0019239 is ok after this commit (with gtk2 2.8 and 2.24) and it looks ok.

Bart Broersma

2012-03-15 17:44

developer   ~0057686

Works OK for me (gtk 2.8.3-4).

Issue History

Date Modified Username Field Change
2011-04-24 10:05 Bernd Kreuss New Issue
2011-04-24 10:05 Bernd Kreuss Widgetset => GTK 2
2011-04-24 11:17 Bernd Kreuss Note Added: 0047732
2011-04-24 12:44 Bernd Kreuss Note Added: 0047734
2011-04-24 12:51 Maxim Ganetsky LazTarget => -
2011-04-24 12:51 Maxim Ganetsky Summary TEdit inserts at wong position when pats of the text are selected => TEdit inserts at wrong position when parts of the text are selected
2011-04-24 12:59 Juha Manninen Note Added: 0047735
2011-04-24 14:04 Bart Broersma Note Added: 0047737
2011-04-24 14:04 Bart Broersma Status new => confirmed
2011-04-24 14:12 Bart Broersma File Added: tedit_selstart.tgz
2011-04-24 14:13 Bart Broersma Note Edited: 0047737
2011-04-24 15:00 Bart Broersma Note Added: 0047738
2011-04-24 15:26 Bart Broersma File Added: gtk2callback.inc.diff
2011-04-24 15:27 Bart Broersma Note Added: 0047739
2011-04-24 15:58 Bernd Kreuss Note Added: 0047740
2011-04-24 16:23 Bernd Kreuss Note Added: 0047741
2011-04-24 16:24 Bernd Kreuss Note Edited: 0047741
2011-04-24 16:25 Bernd Kreuss Note Edited: 0047741
2011-04-25 08:07 Juha Manninen Status confirmed => assigned
2011-04-25 08:07 Juha Manninen Assigned To => Juha Manninen
2011-04-25 08:20 Juha Manninen Fixed in Revision => r30461
2011-04-25 08:20 Juha Manninen Status assigned => resolved
2011-04-25 08:20 Juha Manninen Resolution open => fixed
2011-04-25 08:20 Juha Manninen Note Added: 0047751
2011-04-25 08:58 Bernd Kreuss Note Added: 0047752
2011-04-25 10:26 Bart Broersma Note Added: 0047754
2011-04-25 13:50 Juha Manninen Note Added: 0047755
2011-04-25 14:38 Bernd Kreuss Note Added: 0047757
2011-04-25 17:35 Juha Manninen Note Added: 0047763
2011-04-26 18:08 Bart Broersma Status resolved => assigned
2011-04-26 18:08 Bart Broersma Resolution fixed => reopened
2011-04-26 18:08 Bart Broersma Note Added: 0047805
2011-04-26 18:47 Jesus Reyes Note Added: 0047810
2011-04-26 19:27 Juha Manninen Note Added: 0047815
2011-04-26 19:28 Juha Manninen Assigned To Juha Manninen =>
2011-04-27 22:00 Bart Broersma Note Added: 0047855
2011-04-28 08:04 Bernd Kreuss Note Added: 0047861
2011-04-28 14:23 Bart Broersma Note Added: 0047876
2011-04-28 14:23 Bart Broersma Status assigned => confirmed
2011-04-28 20:20 Bart Broersma Note Added: 0047890
2012-03-14 06:44 Zeljan Rikalo Status confirmed => assigned
2012-03-14 06:44 Zeljan Rikalo Assigned To => Zeljan Rikalo
2012-03-14 11:29 Zeljan Rikalo Note Added: 0057626
2012-03-14 11:29 Zeljan Rikalo Status assigned => feedback
2012-03-14 12:40 Bart Broersma Note Added: 0057635
2012-03-14 15:48 Zeljan Rikalo Note Added: 0057645
2012-03-14 15:48 Zeljan Rikalo Status feedback => confirmed
2012-03-14 17:32 Zeljan Rikalo Fixed in Revision r30461 => 30461,35986
2012-03-14 17:32 Zeljan Rikalo Status confirmed => resolved
2012-03-14 17:32 Zeljan Rikalo Resolution reopened => fixed
2012-03-14 17:32 Zeljan Rikalo Note Added: 0057648
2012-03-14 17:36 Zeljan Rikalo Note Added: 0057649
2012-03-15 17:44 Bart Broersma Note Added: 0057686