Re: NSComboBoxCell bug

[Fred Kiefer]

Hi Quentin,

great that you are going to look into this problem. I must admit that
even after your mail I don't quite understand, why the textDidChange:
call on the text object is needed here.

It really will help, if you find out what Cocoa is doing here.

Cheers
Fred

Quentin Mathé wrote:
> Le 7 mai 06 à 21:58, Fred Kiefer a écrit :
> 
>> Wolfgang Lux wrote:
>>
>>> While this may fix your problem, I'd strongly advise against it.
>>
>> Sorry, I already had hat fix applied, but if you are able to  convince me
>> it does harm, I will take it out again.
> 
> Hi Fred,
> 
> I'm not sure about this one, I have to make some tests, but I think 
> Wolgang is right on the fact this fix isn't correct.
> 
>>> Calling textDidChange: in validateSelection *is* the right thing  to  do
>>> because the contents of the combo box did change after a user   induced
>>> change (which is when textDidChange: should be called  according  to the
>>> docs). Other objects watching the contents of the  combo box may  in
>>> fact
>>> rely on the NSTextDidChangeNotification being  sent.
>>
>> Did you inspect the code that was changed? It is not about any code
>> directly on NSComboBoxCell, the validateSelection method is on the
>> GSComboWindow, this is a fairly internal class. Everything that  goes no
>> here should only be of interest for the internal interaction of  GNUstep.
>> Specificall the code where the textDidChange: call on the text object
>> (again not on the NSComboBoxCell) was removed has a FIXME around  it, as
>> it should not be needed at all. At least Quentin thought so, when he
>> wrote the code.
> 
> I added the following lines :
> 
>       // FIXME: Because NSCell doesn't behave correctly the line  just
> over has
>       // no effect, to correct this fact, the code below is needed.
>       [textObject setString: [_cell _stringValueAtIndex:
>         [_cell indexOfSelectedItem]]];
> 
>       /*
>        * Dispatch the text notifications and by side effect update  the
> cell
>        * object value with the -textDidChange: method of NSTextField 
> which is
>        * the editor delegate
>        */
>       [(NSTextView *)textObject didChangeText];
>       // End of the code to remove
> 
> to workaround the fact [_cell setStringValue: blabla] wasn't  triggering
> textDidChange: notification as it should. When this method  is entered,
> the cell is normally being edited with the field editor.  In this
> precise case, the first problem was the field editor wasn't  updated on
> -setStringValue: and by the way the textDidChange:  notification wasn't
> sent on GNUstep (this doesn't match Cocoa  behavior). Do you know
> whether this bug has been fixed ?
> I haven't fixed this bug when I worked on NSComboBoxCell and related, 
> because it was tricky to implement and no core developers had a  precise
> idea on how to do it iirc.
> 
>> The interaction between the text object and the cell is again totally
>> internal, no other object should be watching for any notifications  here.
>> All the notifications send by the NSComboBoxCell are hopefully 
>> unaffected.
> 
> That's not true. Even if no objects are watching most of the time,  any
> objects can be potentially interested by field editor or cell 
> notifications. _cell is the combobox cell precisely here. If the 
> previous bug hasn't been fixed, NSComboBoxCell notifications are  going
> to be affected, because in this precise case the textDidChange: 
> notification is sent by the field editor (taking care of the combobox 
> cell duty).
> 
>>> The problem really is that NSComboBox's textDidChange: method should
>>> not perform text completion. Instead, text completion should only be
>>> performed when the user has entered text, so I think it would be more
>>> appropriate to override insertText: in NSComboBox and attempt text
>>> completion there.
>>
>> The textDidChange: method is not on the control NSComboBox, but on the
>> cell class NSComboBoxCell.
> 
> Wolfgang was probably referring to NSComboBoxCell here. Otherwise, I 
> think what he suggests is probably the right solution (I'm unable to 
> think of a bad side effect of -insertText: right now).
> 
>> Please, don't get me wrong, I am not saying that the current code, or
>> more specifically the last change is fully correct.
> 
> Noted :-).
> 
>> If you see a
>> problem, please explain it or better, show it with some example code.
> 
> Will know more on the issue after some tests.
> 
> Cheers,
> Quentin.
> 
> -- 
> Quentin Mathé
> qmathe@club-internet.fr
> 
>