Opened 12 months ago

Closed 12 months ago

Last modified 12 months ago

#15540 closed defect (fixed)

wxRichTextCtrlTable: crashes due to an invalid focus object

Reported by: dghart Owned by:
Priority: normal Milestone: 3.0.0
Component: wxRichText Version: stable-latest
Keywords: wxRichTextTable focus crash Cc:
Blocked By: Blocking:
Patch: yes

Description

#15189 fixed crashes from the wxRICHTEXT_CHANGE_OBJECT section of wxRichTextAction::Do when the focused cell was deleted. However I've now found different ways to cause similar crashes. This can be seen in the richtext sample once the patch from #15539 is applied.

Run the sample and click in one of the table's cells. Use the menu to add a new column. Undo it, then type a letter. The sample will assert and crash.

In this instance the problem is that 'container', a richtextcell, is invalid in wxRichTextAction::Do, called by wxRichTextParagraphLayoutBox::InsertTextWithUndo. However the underlying issue is that, whenever a cell has focus, that focus will always become invalid during or after either Undo or Redo, as these swap table instances. Though the details varied with different attempts at solving this, crashes always happened, mostly during invalidation or painting.

Giving the table focus isn't a solution: doing so actually sets focus to cell(0,0). Nor can it be solved by setting the focus elsewhere in the calling code: this works fine at first, but if afterwards the user selects a cell and then does Undo/Redo... The only viable fix that I've found is to set the focus outside the table each time Undo/Redo is called. This results in the caret being located immediately after the table, which is OK if a cell did have focus, and I feel is acceptable even if the focus was elswhere.

richtextbuffer.patch implements this. and reverts the #15189 partial fix.

Attachments (1)

richtextbuffer.patch download (3.6 KB) - added by dghart 12 months ago.

Download all attachments as: .zip

Change History (3)

Changed 12 months ago by dghart

comment:1 follow-up: Changed 12 months ago by juliansmart

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

OK, thanks, applied. We could maybe think of setting the focus to the parent container, rather than the buffer, at some point.

comment:2 in reply to: ↑ 1 Changed 12 months ago by dghart

Replying to juliansmart:

We could maybe think of setting the focus to the parent container, rather than the buffer, at some point.

For the record, I did try that. However, the focused object will be a cell, and both 'container' and the cell's parent container will be the table itself; and setting focus to a table actually gives focus to its first cell, which sooner or later will become invalid.

Maybe I missed something, and there is a more-local object that can safely be focused; but I can't think of one.

Note: See TracTickets for help on using tickets.