Opened 9 months ago

Last modified 8 months ago

#18502 reopened defect

wxPropertyGrid::ChangePropertyValue not work in EVT_PG_CHANGED event handler

Reported by: followait Owned by:
Priority: normal Milestone:
Component: wxPropertyGrid Version: dev-latest
Keywords: Cc: ericj, followait@…
Blocked By: Blocking:
Patch: no

Description

Below is the main code, the VS2019 solution is in attachment.

#include "pch.h"
#include "BuggyPropGrid.h"

BEGIN_EVENT_TABLE(BuggyPropGrid, wxPropertyGrid)
EVT_PG_CHANGED(wxID_ANY, BuggyPropGrid::OnPropChanged)
EVT_PG_DOUBLE_CLICK(wxID_ANY, BuggyPropGrid::OnPGDClick)
END_EVENT_TABLE()

BuggyPropGrid::BuggyPropGrid(wxWindow * parent,
                                   wxWindowID id,
                                   const wxPoint & pos,
                                   const wxSize & size,
                                   long style,
                                   const wxString & name) :
    wxPropertyGrid(parent, id, pos, size, style, name)
{
    Init();
}

void BuggyPropGrid::Init()
{
    _pProp_0 = new wxStringProperty(L"Prop_0");
    _pProp_0->SetValue(5);
    Append(_pProp_0);

    _pProp_1 = new wxStringProperty(L"Prop_1");
    Append(_pProp_1);

    _pProp_2 = new wxStringProperty(L"Prop_2");
    Append(_pProp_2);

    FitColumns();
    
    SetSize(250, 200);
}

void BuggyPropGrid::OnPropChanged(wxPropertyGridEvent & event)
{
    wxPGProperty * pProp = event.GetProperty();
    if (pProp == _pProp_1)
    {
        wxString s = _pProp_1->GetValueAsString();
        double n;
        if (s.ToDouble(&n))
        {
            n *= 2;
            bool ok = ChangePropertyValue(_pProp_2, n); // This line will be executed and return true, but UI doesn't change.
                                                        // wxPropertyGrid::SetPropertyValue works
                                                        // wxPGProperty::SetValue  works
                                                        // But these two doesn't emit change event
            wxASSERT(ok);
        }
    }
}

void BuggyPropGrid::OnPGDClick(wxPropertyGridEvent & event)
{
    unsigned int col = event.GetColumn();
    if (col == 1)
    {
        wxPGProperty * pProp = event.GetProperty();
        if (pProp == _pProp_0)
        {
            ChangePropertyValue(_pProp_1, _pProp_0->GetValue());
        }
    }
}

Attachments (1)

ShowBug.VS2019.7z download (3.9 KB) - added by followait 9 months ago.
VS2019 solution that shows the bug

Download all attachments as: .zip

Change History (10)

Changed 9 months ago by followait

VS2019 solution that shows the bug

comment:1 follow-up: Changed 9 months ago by ericj

  • Cc ericj added
  • Component changed from GUI-all to wxPropertyGrid

The problem occurs if a ChangePropertyValue call is made from inside a wxEVT_PG_CHANGED event handler. The issue is caused by a reentrancy of the method wxPropertyGrid::DoPropertyChanged.

Culprit is the m_inDoPropertyChanged flag which will be set to true by the first ChangePropertyValue call. But when the wxEVT_PG_CHANGED event is sent near the end of this method, and the method is entered again, the flag will still be true and the method will exit immediately.

One solution might be to reset the flag before sending the wxEVT_PG_CHANGED event(s), but i don't know if this could have any unwanted side effects.

comment:2 in reply to: ↑ 1 Changed 9 months ago by followait

  • Cc followait@… added

Replying to ericj:

The problem occurs if a ChangePropertyValue call is made from inside a wxEVT_PG_CHANGED event handler. The issue is caused by a reentrancy of the method wxPropertyGrid::DoPropertyChanged.

Culprit is the m_inDoPropertyChanged flag which will be set to true by the first ChangePropertyValue call. But when the wxEVT_PG_CHANGED event is sent near the end of this method, and the method is entered again, the flag will still be true and the method will exit immediately.

One solution might be to reset the flag before sending the wxEVT_PG_CHANGED event(s), but i don't know if this could have any unwanted side effects.

I think it's better to do some redesign to avoid the problem absolutely.

comment:3 follow-up: Changed 8 months ago by awi

Because ChangePropertyValue sends EVT_PG_CHANGED by itself so preventing from calling it from within EVT_PG_CHANGED handler seems to be a design decision. Perhaps documentation should be updated and this constraint should be explicitly stated.
Changing property value from within the handler is possible by call to SetPropertyValue.

comment:4 Changed 8 months ago by Artur Wieczorek <artwik@…>

In 4489ddc13/git-wxWidgets:

Update wxPropertyGrid documentation

Mention that ChangePropertyValue() shouldn't be called from wxEVT_PG_CHANGED handler.

See #18502.

comment:5 Changed 8 months ago by awi

  • Resolution set to wontfix
  • Status changed from new to closed

comment:6 in reply to: ↑ 3 Changed 8 months ago by followait

  • Resolution wontfix deleted
  • Status changed from closed to reopened

Replying to awi:

Because ChangePropertyValue sends EVT_PG_CHANGED by itself so preventing from calling it from within EVT_PG_CHANGED handler seems to be a design decision. Perhaps documentation should be updated and this constraint should be explicitly stated.
Changing property value from within the handler is possible by call to SetPropertyValue.

I think the functions should keep simple, i.e. ChangePropertyValue = EVT_PG_CHANGED, SetPropertyValue = ChangePropertyValue - EVT_PG_CHANGED.

When functions are simple, it will be easy for developers to use, e.g. avoid deadlock in logic level.

Keep wx cool, thanks.

Last edited 8 months ago by followait (previous) (diff)

comment:7 Changed 8 months ago by vadz

Just a note: several other commonly used functions in wx API (SetValue(), SetSelection()) use the convention that SetXXX() sends the event while ChangeXXX() does not. It would be very confusing if wxPG did the converse with this one.

comment:8 Changed 8 months ago by Artur Wieczorek <artwik@…>

In 4489ddc13/git-wxWidgets:

Update wxPropertyGrid documentation

Mention that ChangePropertyValue() shouldn't be called from wxEVT_PG_CHANGED handler.

See #18502.

comment:9 Changed 8 months ago by Artur Wieczorek <artwik@…>

In 4489ddc13/git-wxWidgets:

Update wxPropertyGrid documentation

Mention that ChangePropertyValue() shouldn't be called from wxEVT_PG_CHANGED handler.

See #18502.

Note: See TracTickets for help on using tickets.