Opened 4 years ago

Closed 5 months ago

#12779 closed defect (fixed)

propgrid sample issues

Reported by: PB Owned by: VZ
Priority: normal Milestone:
Component: samples Version: dev-latest
Keywords: propertygrid Cc: pbfordev@…
Blocked By: Blocking:
Patch: yes

Description

Issue 1

  1. Run propgrid sample.
  1. In the "More Examples" section click on the "AdvImageFileProperty" row - a combobox and a button appear.
  1. Click the button with ellipsis.
  1. Sample asserts.
..\..\src\common\valtext.cpp(130): assert "Assert failure" failed in wxTextValidator::GetTextEntry(): wxTextValidator can only be used with wxTextCtrl or wxComboBox
First-chance exception at 0x751c9617 in propgrid.exe: 0x00001976: 0x1976.

<DBGHELP info removed, I don't think it's relevant here>

Call stack:
[00] wxGUIAppTraitsBase::ShowAssertDialog            c:\dev\desktop\wxwidgets-svn\src\common\appcmn.cpp:502
[01] ShowAssertDialog                                c:\dev\desktop\wxwidgets-svn\src\common\appbase.cpp:1253
[02] wxAppConsoleBase::OnAssertFailure               c:\dev\desktop\wxwidgets-svn\src\common\appbase.cpp:758
[03] wxDefaultAssertHandler                          c:\dev\desktop\wxwidgets-svn\src\common\appbase.cpp:1053
[04] wxOnAssert                                      c:\dev\desktop\wxwidgets-svn\src\common\appbase.cpp:1138
[05] wxTextValidator::GetTextEntry                   c:\dev\desktop\wxwidgets-svn\src\common\valtext.cpp:130
[06] wxTextValidator::Validate                       c:\dev\desktop\wxwidgets-svn\src\common\valtext.cpp:143
[07] wxPropertyGrid::DoEditorValidate                c:\dev\desktop\wxwidgets-svn\src\propgrid\propgrid.cpp:3535
[08] wxPropertyGridInterface::EditorValidate         c:\dev\desktop\wxwidgets-svn\src\propgrid\propgridiface.cpp:784
[09] wxPGEditorDialogAdapter::ShowDialog             c:\dev\desktop\wxwidgets-svn\src\propgrid\editors.cpp:2131
[10] wxPropertyGrid::HandleCustomEditorEvent         c:\dev\desktop\wxwidgets-svn\src\propgrid\propgrid.cpp:3639
[11] wxPropertyGridEditorEventForwarder::ProcessEvent        c:\dev\desktop\wxwidgets-svn\src\propgrid\propgrid.cpp:3916
[12] wxEvtHandler::SafelyProcessEvent                c:\dev\desktop\wxwidgets-svn\src\common\event.cpp:1493
[13] wxWindowBase::HandleWindowEvent                 c:\dev\desktop\wxwidgets-svn\src\common\wincmn.cpp:1306
[14] wxControl::ProcessCommand                       c:\dev\desktop\wxwidgets-svn\src\msw\control.cpp:313
[15] wxButton::SendClickEvent                        c:\dev\desktop\wxwidgets-svn\src\msw\button.cpp:887
[16] wxButton::MSWCommand                            c:\dev\desktop\wxwidgets-svn\src\msw\button.cpp:916
[17] wxWindow::HandleCommand                         c:\dev\desktop\wxwidgets-svn\src\msw\window.cpp:5284
[18] wxWindow::MSWWindowProc                         c:\dev\desktop\wxwidgets-svn\src\msw\window.cpp:3091
[19] wxWndProc                                       c:\dev\desktop\wxwidgets-svn\src\msw\window.cpp:2768
[20] IsThreadDesktopComposited               

Issue 2

  1. In section "Appearance" expand the "Font" block.
  1. In the "Face Name" row click on the combobox arrow to display the font face names available.
  1. Attempt to select any font name.
  1. On Windows 7 the font for the dialog changes after confirming the change, but the change is not reflected in the font properties displayed in the grid. It sometimes asserts, but very rarely - it took me great many attempts to reproduce it. I couldn't find the pattern, it doesn't seem to be a font name / combobox index. On Windows XP the very same executable asserts every single time I attempt to change font face no matter what font I choose.
c:\Dev\Desktop\wxWidgets-SVN\include\wx/propgrid/property.h(789): assert "i < GetCount()" failed in wxPGChoicesData::Item(): invalid index
First-chance exception at 0x751c9617 in propgrid.exe: 0x00001976: 0x1976.

<DBGHELP info removed, I don't think it's relevant here>

Call stack:
[00] wxGUIAppTraitsBase::ShowAssertDialog            c:\dev\desktop\wxwidgets-svn\src\common\appcmn.cpp:502
[01] ShowAssertDialog                                c:\dev\desktop\wxwidgets-svn\src\common\appbase.cpp:1253
[02] wxAppConsoleBase::OnAssertFailure               c:\dev\desktop\wxwidgets-svn\src\common\appbase.cpp:758
[03] wxDefaultAssertHandler                          c:\dev\desktop\wxwidgets-svn\src\common\appbase.cpp:1053
[04] wxOnAssert                                      c:\dev\desktop\wxwidgets-svn\src\common\appbase.cpp:1129
[05] wxPGChoicesData::Item                           c:\dev\desktop\wxwidgets-svn\include\wx\propgrid\property.h:789
[06] wxPGChoices::Item                               c:\dev\desktop\wxwidgets-svn\include\wx\propgrid\property.h:1014
[07] wxPGChoices::GetLabel                           c:\dev\desktop\wxwidgets-svn\include\wx\propgrid\property.h:967
[08] wxSystemColourProperty::ColourToString          c:\dev\desktop\wxwidgets-svn\src\propgrid\advprops.cpp:1154
[09] wxSystemColourProperty::ValueToString           c:\dev\desktop\wxwidgets-svn\src\propgrid\advprops.cpp:1181
[10] wxPGProperty::GetValueAsString                  c:\dev\desktop\wxwidgets-svn\src\propgrid\property.cpp:1002
[11] wxPGProperty::GetDisplayedString                c:\dev\desktop\wxwidgets-svn\include\wx\propgrid\property.h:1675
[12] wxPGProperty::GetDisplayInfo                    c:\dev\desktop\wxwidgets-svn\src\propgrid\property.cpp:783
[13] wxPGDefaultRenderer::Render                     c:\dev\desktop\wxwidgets-svn\src\propgrid\property.cpp:222
[14] wxPropertyGrid::DoDrawItems                     c:\dev\desktop\wxwidgets-svn\src\propgrid\propgrid.cpp:2501
[15] wxPropertyGrid::DrawItems                       c:\dev\desktop\wxwidgets-svn\src\propgrid\propgrid.cpp:1994
[16] wxPropertyGrid::OnPaint                         c:\dev\desktop\wxwidgets-svn\src\propgrid\propgrid.cpp:1844
[17] wxAppConsoleBase::HandleEvent                   c:\dev\desktop\wxwidgets-svn\src\common\appbase.cpp:589
[18] wxAppConsoleBase::CallEventHandler              c:\dev\desktop\wxwidgets-svn\src\common\appbase.cpp:601
[19] wxEvtHandler::ProcessEventIfMatchesId           c:\dev\desktop\wxwidgets-svn\src\common\event.cpp:1292
[20] wxEventHashTable::HandleEvent                   c:\dev\desktop\wxwidgets-svn\src\common\event.cpp:935

Also when you change font face name using a keyboard (e.g. pressing "v" to select the first name starting with "v") "..\..\src\msw\window.cpp(674): 'SetFocus' failed with error 0x00000057 (invalid parameter)." is output in the log window (in the debug build).

Issue 3

Also when you attempt to change any individual font property in the "More Examples"/"Font Data Property", the font face changes to Arial (from the default Segoe UI on Windows 7 or Tahoma on Windows XP) no matter what you change there and can't be changed to anything else. Changing font in this section works if you invoke Font Dialog using the button with ellipsis on the row where all the font attributes are displayed at once.


wxWidgets: 2.9-svn rev 66383, both library and samples were built using:
nmake -f makefile.vc BUILD=debug RUNTIME_LIBS=static UNICODE=1
Compiler: Visual C++ 2008 Express
OS: Windows 7 Professional Czech, Windows XP Professional Czech (Windows XP mode on Windows 7)

Attachments (1)

Fixes-bugs-in-propgrid-sample.patch download (1.2 KB) - added by awi 6 months ago.
Patch fixing bugs in wxFontDataProperty.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 4 years ago by _Cool-

Issue 4
Also don't work select colour field with wxColourProperty class if i'm selecting it in combobox. It's changing to my colour, but then restore old value.

Issue 5
Methods GetVal and ColToInd in wxSystemColourProperty must be virtual?

comment:2 Changed 4 years ago by jmsalli

  • Status changed from new to confirmed

Thanks for your reports, will look into this.

comment:3 Changed 4 years ago by JMS

(In [66385]) Instead of having wxComboCtrl mimic wxTextEntry interface, make it actually inherit from the class and implement functions to redirect to the embedded wxTextCtrl. This allows us to simplify the code and get rid of the dirty trick of directing wxComboCtrl's validator to the embedded wxTextCtrl. Also see #12779, which issue 1 is fixed by this change.

comment:4 follow-up: Changed 3 years ago by oneeyeman

I'm testing the propgrid sample from rev.70748 on Windows 7 64-bit with MSVC2010.
Issue #2 is gone completely.
Issue #3 still stands. Font is changed to Arial.
Issue #4 is gone. Just try to change "Appearance->Cell Colour". It changes and stays.
Don't know about #5.

So the only 2 are #3 and #5.

comment:5 in reply to: ↑ 4 ; follow-up: Changed 3 years ago by PB

Unfortunately, issue no. 2 still manifests in the exact same way as described in my original post on Windows XP (I will try it on Windows 7 later in the day).

Testing environment:
wxWidgets r70543
MS Visual C++ 2008 Express
Windows XP Professional (Czech language and locale) SP3 with all updates installed

comment:6 Changed 3 years ago by vadz

It would be great if you could debug it further and propose fixes for this code. Its original author has unfortunately disappeared so we really need help with maintaining it.

comment:7 in reply to: ↑ 5 Changed 3 years ago by PB

  • Cc pbfordev@… added

Replying to PB:

Unfortunately, issue no. 2 still manifests in the exact same way as described in my original post on Windows XP (I will try it on Windows 7 later in the day).

Mm, I just tested it on Windows 7 32-bit Professional Czech and it also behaves just like described in the ticket.

I've also noticed another issue, when the pop-up window with font names (and few other like that, some other work as they should) is shown, after pressing certain keys (like "t" or "a") that window doesn't behave like a proper combobox pop-up. Instead of scrolling to and focusing the first item starting with the letter, the window is closed as if confirmed.

It's weird that the original issue no.2 appears to be fixed on oneeyeman's computer, compiler aside, the difference being only the 32-bit and 64-bit and locale/language.

comment:8 Changed 3 years ago by PB

I've spent some time looking into issue no. 2.
The reason for the crash/assert is (I believe that the problem is not in the sample but the library component itself):

  1. wxFontProperty private child windows are implemented using wxEnumProperty.
  2. wxSystemColourProperty is derived from wxEnumProperty.
  3. wxEnumProperty has a static class member ms_nextIndex;
  4. wxSystemColour has about 25 items, wxFontProperty's child with font face names has usually great many more (more or less equals the number of fonts installed on a given system)
  5. When you select a font face item with an index greater than the number of colours in the wxSystemColour properties, the left-over value of static variable ms_nextIndex (the one of font face name combobox index) is used, thus the assert/crash when querying label name. I have no idea why crash/assert happens only rarely on Windows 7 but always on Windows XP.
  6. wxEnumProperty::ValueFromString_() fails to set the proper string value for a chosen font for a private child with font face names, that is why the change in the font face is not reflected in the control, which may also lead to the issue in the point above.

I can't figure out why ms_nextIndex was made to be static. When I made few simple changes to wxEnumProperty code everything seemed to work just fine and nothing appeared to be broken. But there must be some reason why the variable was declared static and thus shared by all wxEnumProperty (and derived) instances, I can't just see it, so I gave up.

comment:9 Changed 3 years ago by vadz

Unfortunately I don't understand this code at all and ms_nextIndex and all manipulations with it are a complete mystery to me too. If you can update the code to avoid using it and still work, please consider submitting a patch doing it.

comment:10 Changed 6 months ago by vadz

  • Resolution set to fixed
  • Status changed from confirmed to closed

See the soon-to-be-closed #15449.

comment:11 Changed 6 months ago by PB

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Version changed from stable-latest to dev-latest

Thank you for the fix.

Unfortunately, issue 3 does not seem to be fixed - Font Data Property in the propgrid sample still behaves as described in the ticket. Does it work as it should for others?

comment:12 Changed 6 months ago by awi

  • Keywords propertygrid added
  • Patch set

Fortunately, issue 3 is a result of bugs in progrid sample only. Problems are caused by derived class wxFontDataProperty which contains few bugs.
Patch fixing this issue is attached.

Changed 6 months ago by awi

Patch fixing bugs in wxFontDataProperty.

comment:13 Changed 5 months ago by VZ

  • Owner set to VZ
  • Resolution set to fixed
  • Status changed from reopened to closed

In 76596:

Fix changing individual font properties in the propgrid sample.

Fixes the bug in the sample which resulted in the UI not working as expected.

Closes #12779.

Note: See TracTickets for help on using tickets.