#15224 closed defect (fixed)

wxRichTextTable: Setting a cell's text colour affects subsequent cells

Reported by: dghart Owned by:
Priority: normal Milestone: 3.0.0
Component: wxRichText Version: stable-latest
Keywords: wxRichTextCell wxRichTextBox SetTextColour() Cc:
Blocked By: Blocking:
Patch: yes

Description

The attached patch to the 'richtext' sample sets cell(1,1)'s text colour red. As you can see, the text in all downstream cells also becomes red. This seems to be a drawing issue: the xml output is the correct:

 <cell textcolor="#FF0000">

for that cell only.

In my own code I've tried various workarounds, including:

wxRichTextObjectList::compatibility_iterator node = cell.GetChildren().GetFirst();
while (node)
  { wxRichTextParagraph* para = wxDynamicCast(node->GetData(), wxRichTextParagraph);
    if (para)
      { wxRichTextObject* child = para->GetChild(0);
        if (child) child->GetAttributes().Apply(attr);
      }

    node = node->GetNext();
  }

which results in the correct xml:

 <cell>
    <paragraph>
      <text textcolor="#FF0000">foo</text>
    </paragraph>
 </cell>

but the colour still runs.

In contrast, setting a cell's background colour works correctly.

Attachments (6)

richtext.diff download (561 bytes) - added by dghart 15 months ago.
NoSelection.png download (31.4 KB) - added by dghart 11 months ago.
WithSelection.png download (22.4 KB) - added by dghart 11 months ago.
AfterPatch.png download (30.5 KB) - added by dghart 11 months ago.
patch.diff download (2.5 KB) - added by dghart 11 months ago.
patch1.diff download (1.7 KB) - added by dghart 11 months ago.

Download all attachments as: .zip

Change History (13)

Changed 15 months ago by dghart

comment:1 Changed 14 months ago by vadz

  • Milestone changed from 2.9.5 to 3.0

This is not 2.9.5 critical, even if it would be great to apply it as soon as possible to get more testing for these patches. But I leave the decision as to whether do it right now (before 2.9.5) or later to Julian.

comment:2 Changed 14 months ago by vadz

This is not 2.9.5 critical, even if it would be great to apply it as soon as possible to get more testing for these patches. But I leave the decision as to whether do it right now (before 2.9.5) or later to Julian.

comment:3 Changed 14 months ago by vadz

This is not 2.9.5 critical, even if it would be great to apply it as soon as possible to get more testing for these patches. But I leave the decision as to whether do it right now (before 2.9.5) or later to Julian.

comment:4 Changed 11 months ago by dghart

  • Keywords wxRichTextBox added
  • Patch set

More information, and a second-rate fix.

First, this leak of ink colour is the cause of a different issue that I'd noticed: if some text is selected in a cell, downstream text in this and later cells becomes invisible. This is because (for me) selected text is drawn white-on-blue; and the leaking white ink shows poorly against my default white background.

Second, this issue also affects wxRichTextBox itself. This can be demonstrated in the richtext sample. As seen in the first two screenshots, selecting text in the wxRichTextBox hides the rest of it, and also in the cells in the table (you may need to scroll to force a refresh before this happens). Give a cell a non-white background and the white ink becomes visible.

On investigation it turns out that this happens only if the foreground colour in the control is invalid, which it is by default. (The background colour is invalid too, but this seems not to matter.) So patch.diff applies the GetBasicStyle() foreground colour to the controls in wxRichTextCtrl::WriteTextBox and wxRichTextCtrl::WriteTable; and, for those of us who call it direct, in wxRichTextTable::CreateTable too.

The third screenshot shows that this fix works in the sample, and it works for me too. Of course understanding and removing the cause of the leak would be a much better solution...

Changed 11 months ago by dghart

Changed 11 months ago by dghart

Changed 11 months ago by dghart

Changed 11 months ago by dghart

comment:5 Changed 11 months ago by juliansmart

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

I can't think of a better way of doing this yet, so I've committed this patch - thanks!

comment:6 Changed 11 months ago by dghart

  • Resolution fixed deleted
  • Status changed from closed to reopened

Thanks for committing this, but I've just realised I missed something: wxRichTextTable::AddColumns and AddRows need the same fix. patch1.diff does so.

Changed 11 months ago by dghart

comment:7 Changed 11 months ago by juliansmart

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

Thanks - now committed.

Note: See TracTickets for help on using tickets.