[SOLVED] TCombobox causes Access Violation - Is my fix causing new problems?

[Hansaplast]

In one of my more complicated and larger projects, I keep running into Access Violations at times when opening a TCombox.

In my code the list items change under certain circumstances - so I figured that would be the culprit.
Even disabling the control while changing the items list made it still crash.
Disabling all events still keeps it crashing and the debugger keeps point to a certain procedure in "CocoaTextEdits".

Code: Pascal  [Select][+][-]

1. procedure TCocoaReadOnlyComboBoxMenuDelegate.menu_willHighlightItem(
2.   menu: NSMenu; item: NSMenuItem);
3. begin
4.   if Assigned(_lastHightlightItem) then
5.     _lastHightlightItem.view.setNeedsDisplay_( True );
6.   _lastHightlightItem:= item;
7. end;  

With the debugger stating that _lastHightlightItem is not defined (of course very likely an incorrect statement due to the crash).

I've tried numerous little test projects to see if I can reproduce this, but cannot reproduce it in those little projects.
Doing all kinds of elimination steps in my project (assuming I did something wrong) didn't fix it either.
(I'm still under the impression that I made a mistake)

However, what did fix it is commenting out the first two lines in "TCocoaReadOnlyComboBoxMenuDelegate" (note: the TComboBox is not set to be readonly - in case that is relevant), like so:

Code: Pascal  [Select][+][-]

1. procedure TCocoaReadOnlyComboBoxMenuDelegate.menu_willHighlightItem(
2.   menu: NSMenu; item: NSMenuItem);
3. begin
4. //  if Assigned(_lastHightlightItem) then
5. //   _lastHightlightItem.view.setNeedsDisplay_( True );
6.   _lastHightlightItem:= item;
7. end;  

I am confident that these two lines have a very legit purpose, so I created a few test projects to see what would break because of removing these 2 lines, and low and behold, everything I tested works just fine.

Now I'm quite confident that the dev who wrote this code, placed it there for a very valid reason.

Note: I'm aware that posting my code would be more helpful, ut it is such a large project that it would be hard for anyone to see where I may have made a mistake. On top of that: it would be my problem to figure out of course.

After digging through all my code most of the day now, I just wanted to make sure I'm not wasting my time.

So my question is: did I bump into a bug or no longer needed code in "CocoaTextEdits"?

[Zvoni]

My guess is a Bug ("dangling" _lastHightlightItem).
Have you tried to first explicitely clear the Items of the ComboBox (Items is a TStrings), and THEN adding the new Items?
Just as an Try and Error.

It sounds like, e.g. Item 6 is Highlighted, and then you change (!!) the list (not clearing it), which then consists of only 3 items.
And _lastHightlightItem still points to the non-existing item.

my 2 cents

[Hansaplast]

Good suggestion, and tried it right away ... (combo.items.clear)
Unfortunately, still the same crash 😞
Keeping the clear in the code though. Seems like the right thing to do.

Very much appreciate your 2 cents though - been doing trial and error for most of the day now.
Commenting out was the only thing that worked and did not seem to have an effect at all on how TComboBox works in other scenarios.

How the one who coded this reads this and might have an idea.

[rich2014]

just confirm，you are using a TCombobox with ReadOnly=True, right?

please create a new issue and provide a simple test project (even if it is so simple that it does not crash)

[Hansaplast]

Hi Rich2014,
Thanks for chiming in    ...



Actually ... no.

Initially I had it set to ReadOnly := true, but I changed that to ReadOnly := False once I noticed the "ReadOnly" part in the procedure name figuring it would not use this procedure.
Changing the value does not appear to change the error (been testing with ReadOnly := False).

As mentioned before:
I have not been able to make a simple demo project that reproduces this (hence my assumption I am doing something wrong in my code). Been digging into this for a day now (in my own code) and noticed that the only thing that fixes this is disabling these 2 lines.

I assume I am doing something wrong, but since commenting out those two lines did not break TComboBox in other projects, I was wondering if this code still has a purpose.

Not sure what you mean with "a simple test project (even if it is so simple that it does not crash)" ...?

[Zvoni]

Since you're on Cocoa: Could you check if there is a (public) Property LastHighlightItem or similiar?
the leading underscore in your procedure suggests a Private Field. No idea if you can access it from outside.
If yes, i would check it before you clear/clearSelection the ComboBox, if it has a valid value, then clear/clearselection the ComboBox, then check again (and it should AV then).

If you find something like that, you could remove the commenting in above code, but explicitely setting LastHighlightItem to Nil (or whatever value) before clearing the combobox

[Hansaplast]

Thanks Zvoni for chiming in 

Looks like the TComboBox in Cocoa is using a popupmenu (NSMenu) for its dropdown (TCocoaReadOnlyComboBoxMenuDelegate).

_lastHightlightItem is a private property of TCocoaReadOnlyComboBoxMenuDelegate, which in turn is a private property of (_menuDelegate) of TCocoaReadOnlyComboBox.

Not sure how to access that properly from my own code.
I cannot find a public "LastHighlightItem" or anything like it.

Also not sure what "ReadOnly" implies in the name TCocoaReadOnlyComboBox.
The ComboBox is marked as ReadOnly:=False, but of course this could be named as such for other reasons.

[Zvoni]

Also not sure what "ReadOnly" implies in the name TCocoaReadOnlyComboBox.
The ComboBox is marked as ReadOnly:=False, but of course this could be named as such for other reasons.

Well, "ReadOnly" implies that you can't change anything in it.
Maybe you really use the wrong control

From here: https://wiki.freepascal.org/Cocoa_Internals/Text_Controls

Non-readonly ComboBox is implemented via NSComboBox type. Readonly combobox is implemented via NSPopupButton

[Hansaplast]

Maybe you really use the wrong control
From here: https://wiki.freepascal.org/Cocoa_Internals/Text_Controls

Non-readonly ComboBox is implemented via NSComboBox type. Readonly combobox is implemented via NSPopupButton

As mentioned in the title; I am using a standard LCL TComboBox.
That is the only thing I use. Nothing Cocoa specific.
I wouldn't even know where to find the ReadOnly TCombobox, unless it refers to the "ReadOnly" property of the TCombobox (which in my case is set to false) or the fact that the "Style" property is set to csDropDownList.

Note: The Access Violation however traces back to the Cocoa implementation of TComboBox.

[rich2014]

as Zvoni said, you should set something wrong.

provide a simple test project, no matter it's crash or not.

Actually ... no.

Initially I had it set to ReadOnly := true, but I changed that to ReadOnly := False once I noticed the "ReadOnly" part in the procedure name figuring it would not use this procedure.

[Hansaplast]

Narrowed it down to Style := csDropDownList.

Only then TComboBox crashes (the appearance and position op the dropdown is different then as well - making me think this is where a Menu is being used and seen as "ReadOnly").

Using Styles "csSimple" and "csDropDown" work just fine - I just want the user to be limited to the options in the dropdown only.

Managed to build a crashing test project now that I figured this out (attached).

To make it crash:

Run the project.
Click the "Add Lines" button, and select an item from the dropdown list.
Next click the "Replace Text" button, and try to select an item - crashes the minute the dropdown list opens.

Doesn't matter if I use Items.Append() or Items.Text:= ... it just crashes when the dropdown list content was changed.

Also seeing now why commenting out the two lines in TCocoaReadOnlyComboBoxMenuDelegate is not a good idea 😉 (kinda assumed this already, hence my question here - it keeps an item selected even though it changed).

[rich2014]

@Hansaplast

test with the last Main Trunk please.

[Hansaplast]

Hi Rich2014! Thanks for trying to help out!

I tried: Lazarus 4.99 (rev main_4_99-1960-gb2a22bea0d) FPC 3.3.1 x86_64-darwin-cocoa

With my little example project (deleted all project files and dirs except project1.lpi, project1.lpr, unit1.lfm, and unit1.pas) and did a full clean+build.
Still the same error.

With my own (large)project: Same error as well.

[rich2014]

it's not the last Main Trunk.

the commit is 4644ad9 (Cocoa: fix the issue in TComboBox (readonly))

I tried: Lazarus 4.99 (rev main_4_99-1960-gb2a22bea0d) FPC 3.3.1 x86_64-darwin-cocoa

[Hansaplast]

Oh geez, I'm so sorry Rich2014 - my bad.
I had checked the "rebuild only" option (a tiny well hidden checkbox in FPCUpDeluxe) because of linker issues.

Tried it with main_4_99-2176-g15a44331c3, but the compiler crashes on something else:

Code: [Select]

(3104) Compiling synhighlighterhashentries.pas
(3104) Compiling synhighlighterpython.pas
/Users/hans/lazarus/fpcupdeluxe/lazarus/components/synedit/synhighlighterpython.pas(549,41) Error: (3035) Range check error while evaluating constants
/Users/hans/lazarus/fpcupdeluxe/lazarus/components/synedit/synhighlighterpython.pas(1250) Fatal: (10026) There were 1 errors compiling module, stopping
make[1]: *** [lazbuildlclpackages] Error 2
make: *** [lazbuild] Error 2
Fatal: (1018) Compilation aborted