Bug 156685 - Default font color in tables is white over white background (font color should be black)
Summary: Default font color in tables is white over white background (font color shoul...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
24.2.0.0 alpha0+
Hardware: All All
: high normal
Assignee: Heiko Tietze
URL:
Whiteboard: target:24.2.0
Keywords: bibisected, bisected, regression
: 142799 (view as bug list)
Depends on:
Blocks: ImpressDraw-Tables
  Show dependency treegraph
 
Reported: 2023-08-08 23:00 UTC by Rafael Lima
Modified: 2023-10-20 23:04 UTC (History)
9 users (show)

See Also:
Crash report or crash signature:


Attachments
Text in presentation notes (141.00 KB, application/vnd.oasis.opendocument.presentation)
2023-08-10 17:53 UTC, Regina Henschel
Details
Wrong automatic font color in text box in group in docx (23.13 KB, application/vnd.openxmlformats-officedocument.wordprocessingml.document)
2023-09-08 12:00 UTC, Regina Henschel
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Rafael Lima 2023-08-08 23:00:28 UTC
For some reason, in master (24.2) the default font color of tables in Impress changed to white. It should be black (as it has always been).

Steps to reproduce
0) Open Impress on a clear profile, with default settings
1) Create a blank presentation in Impress
2) Insert a table (Insert - Table) with any size
3) Type something inside the table
4) Notice that the font color is white over a white background

This seems a regression to me. It appears the default table template has changed to using white font color, but I have to check it out.

Tested with

Version: 24.2.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 3ce30d6a1ea1cd3e392cb21fa37102795578eeb7
CPU threads: 16; OS: Linux 6.2; UI render: default; VCL: kf5 (cairo+xcb)
Locale: pt-BR (pt_BR.UTF-8); UI: en-US
Calc: threaded
Comment 1 Regina Henschel 2023-08-09 17:23:09 UTC
I see two of errors:

The table template refers a <style:style> element of family "table-cell" with name "bg-none". That style has no child element <style:text-properties>. So the property has to be searched in the parent style. The parent-style has the name "default". But such style does not exist in the file.

Try: Add <style:text-properties fo:color="#ff0000"/>. And indeed, now the text is red.

Next try: Instead add <style:text-properties style:use-window-font-color="true"/>. Now the font color should be black. But that is not the case, even not if I force the cell background to White instead of Transparent.

It is OK in Version: 7.5.2.2 (X86_64) / LibreOffice Community
Build ID: 53bb9681a964705cf672590721dbc85eb4d0c3a2
CPU threads: 8; OS: Windows 10.0 Build 19045; UI render: Skia/Raster; VCL: win
Locale: de-DE (en_US); UI: en-US
Calc: CL threaded
Comment 2 Regina Henschel 2023-08-09 17:47:19 UTC
More narrow interval:
Good on Version: 24.2.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: c7d202a61f9ce81b76b29e61252c23aa66daff07
CPU threads: 8; OS: Windows 10.0 Build 19045; UI render: Skia/Raster; VCL: win
Locale: en-US (en_US); UI: en-US
Calc: CL threaded

Bad on Version: 24.2.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 6256d5fe2e7cb1bb002d5fe59527d3a3fbf6963f
CPU threads: 8; OS: Windows 10.0 Build 19045; UI render: Skia/Raster; VCL: win
Locale: en-US (en_US); UI: en-US
Calc: CL threaded
Comment 3 Regina Henschel 2023-08-09 19:25:43 UTC
It comes from commit https://cgit.freedesktop.org/libreoffice/core/commit/?id=ddb483509113e469b771320fea52f1b089574021

If I set the value back to 62, I get a black font color.

I have no idea why 62 works but 156 not.
Comment 4 Regina Henschel 2023-08-09 20:39:31 UTC
Some more findings:

SdrObject::setSuitableOutlinerBg creates the value from the background fill of the shape. And because the table object has no own style, the default style is used. That has currently RGB(114,159,207). Then this is used in IsDark() in tools.hxx. But for RGB(114,159,207) the GetLuminance returns 151 and therefore the table is considered as "dark".

So the error is not directly the mentioned commit, but the fact that the outliner does not use the actual table cell background but the SdrObject fill color.

To test it, create an own new graphic style with an area fill color and assign it to the table. This does not change the table cell background. Now vary the area fill color, e.g. use a gray of 156 or a gray of 157.

That error exist already in LO7.5. If the font color is set to automatic and you change the cell background to black or to a dark color, the text does not become white as it should for "automatic".
Comment 5 Regina Henschel 2023-08-10 17:53:06 UTC
Created attachment 188918 [details]
Text in presentation notes

Not only text in tables is affected by the new threshold, but presentation notes as well.

Open the attached presentation.
Go to Notes view.
Browse through the pages. You will notice, that the text of the notes is White on White and text in the thumbnails is White on White too. If you do the same with LO 7.5 you can see these texts in black.

If the underlying root cause cannot be fixed quickly, the threshold should be set so, that our default object fill color is not considered to be 'dark'.
Comment 6 Rafael Lima 2023-08-17 12:34:14 UTC
(In reply to Regina Henschel from comment #5)
> Browse through the pages. You will notice, that the text of the notes is
> White on White and text in the thumbnails is White on White too. If you do
> the same with LO 7.5 you can see these texts in black.

I can confirm this behavior in:

Version: 24.2.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 926c5246b6694d469a6caed5d7ea4c3a68648468
CPU threads: 16; OS: Linux 6.2; UI render: default; VCL: kf5 (cairo+xcb)
Locale: pt-BR (pt_BR.UTF-8); UI: en-US
Calc: CL threaded

@Regina, thanks for your thorough analysis of the problem. Indeed RGB(114,159,207), the default blue-ish color used by "Default Drawing Style" is now considered to be dark and then the text color becomes white.

The problem also appears if:
1) Create a new Impress presentation
2) Add any shape into it (f.i. a rectangle); by default it will use the "Default Drawing Style"
3) Notice the text will be white (previously it was black)

@Heiko, any thoughts on this?
Comment 7 Heiko Tietze 2023-08-18 09:29:43 UTC
White text color on sky blue background is an improvement. But tables should not draw white on white. The style defines fo:background-color="#ffffff" and use-window-font-color="true" and I suspect this as the root cause. What do you think, Maxim?
Comment 8 Regina Henschel 2023-08-18 13:22:59 UTC
The background for text in the cell is determined in
Color GetTextEditBackgroundColor(const SdrObjEditView& rView)
https://opengrok.libreoffice.org/xref/core/svx/source/svdraw/svdetc.cxx?r=2c8c436c#673

That looks with GetDraftFillColor() for a color in the item set of the cell. But that has no XATTR_FILLSTYLE item. That seems to be a missing feature of Draw-tables compared to Writer-tables. For example the table properties have no drop-down-field to select "Cell, Row, Table" in the Background tab as known from Writer-tables.
GetTextEditBackgroundColor then looks in the MergedItemSet. That has the fill attributes of the table object and results the default background color.

I have not looked what happens in case of Impress notes and thumbnails, which are affected too.

I guess, that all these cannot be fixed quickly. And who will do it? Therefore I suggest to use a threshold, for which the default background color is not considered "dark" until the underlying problems are all solved.
Comment 9 Myndex 2023-08-21 23:29:27 UTC
Hi Regina, Heiko, Rafael,

Sorry for my delay in response, and sorry this is causing trouble from the other bug being fixed.

To highlight the importance of corrections for the underlying issues:

For color RGB(114,159,207), white is higher contrast than black. Depending on the size of the font this can be quite important to readability and visual accessibility.

FWIW I have been practically unable to use LibreOffice because of how text colors are unreadable when my OS is in dark mode, so getting some of these issues corrected is important for the LibreOffice UX overall.


The present bug

Instead of lowering the threshold, could the default blue be lightened? That seems like a proactive step in the right direction.



Notes on contrast & methods:

The WCAG 2 contrast math is incorrect, particularly for dark colors. An APCA-based calculator gives useful results over the visual range. https://www.myndex.com/APCA/

If the auto color is ONLY white or back, then changing the math is not needed (probably). The existing but archaic method is fine for a single flip point. The 156 came from testing and aligning with perceptually uniform methods, and exactitude is not needed. RGB(114,159,207) with black text rates a contrast of Lc 50, which is lower than ideal, okay for larger / bolder fonts (32px normal, 21px bold) if readability is important.

RGB(114,159,207) is close to contrast center, so it's perhaps black text is not a show-stopper, but white text has about 15%-20% more readable contrast.


Perception and Flips

To keep text black with a perceptually similar blue, rgb(125,179,232) is a reasonable minimum, rgb(135,197,255) is better for black text, and rgb(158,206,255) is preferred for smaller black text.


Dark mode (white text) perceptually uniform inversions:

rgb(112,155,201) reasonable minimum
rgb(96,136,178)  better for white text
rgb(90,125,156) preferred for smaller white text


Thank you for reading

Andy
Comment 10 raal 2023-08-23 06:57:38 UTC
(In reply to Regina Henschel from comment #3)
> It comes from commit
> https://cgit.freedesktop.org/libreoffice/core/commit/
> ?id=ddb483509113e469b771320fea52f1b089574021
> 
> If I set the value back to 62, I get a black font color.
> 
> I have no idea why 62 works but 156 not.

setting as bibisected
Comment 11 Heiko Tietze 2023-08-23 10:59:28 UTC
(In reply to Regina Henschel from comment #8)
> The background for text in the cell is determined in
> Color GetTextEditBackgroundColor(const SdrObjEditView& rView)...

In fact XATTR_FILLSTYLE for the table object is FillStyle_SOLID and by that defaults to Tango Sky Blue 114,159,207. Removing this calculation temporary (ie. bRetval = false) detects the correct color. Tried to solve with rSet.Put(XFillStyleItem (drawing::FillStyle_NONE)); in SdDrawDocument::CreateDefaultCellStyles() but no success. Another idea was to read the cell background in GetTextEditBackgroundColor() at if( pTable ) - but no idea how to.

Changing the threshold to any other value would be a bad bandaid. The new TS draws for example black font on black first row in case of the second style.
Comment 12 Regina Henschel 2023-08-23 11:58:16 UTC
(In reply to Heiko Tietze from comment #11)

> Changing the threshold to any other value would be a bad bandaid. The new TS
> draws for example black font on black first row in case of the second style.

Do you mean table style "black-dark"? That has no problem in the first row, because the font color is not "automatic" but it is set to "White".

A table style with an explicit cell background color can and should set a text color in that cell style, which fits to the cell background. Those cases are not a problem.

I plead to use a threshold of 150 until all the problematic cases are solved. Currently known are: table-shapes, notes, thumbnails, data-tables

Or follow the suggestion of Myndex and change the default shape fill color to a value, for which IsBlack() is false. That is COLOR_DEFAULT_SHAPE_FILLING define in include/svx/xdef.hxx
Comment 13 Regina Henschel 2023-08-23 12:14:51 UTC
IsBlack() --> IsDark()
Comment 14 Heiko Tietze 2023-08-24 09:41:19 UTC
(In reply to Regina Henschel from comment #12)
> Or follow the suggestion of Myndex and change the default shape fill color
> to a value, for which IsBlack() is false.
"Tango Sky Blue" is used a gazillion times in qa. And it wont be a solution since users end up in white on white when changing the table properties.
Comment 15 Regina Henschel 2023-08-24 09:51:47 UTC
I too do not see a different default fill color as solution. I have tested that it works only for new documents. If a document was created in a version before LO 24.2, you will still get white text on white cell background, because the document contains the default value that exist at that time.

Thus I still plead for using threshold 150 instead of 156.
Comment 16 Mike Kaganski 2023-08-24 09:53:45 UTC
(In reply to Heiko Tietze from comment #11)
> In fact XATTR_FILLSTYLE for the table object is FillStyle_SOLID and by that
> defaults to Tango Sky Blue 114,159,207. Removing this calculation temporary
> (ie. bRetval = false) detects the correct color. Tried to solve with
> rSet.Put(XFillStyleItem (drawing::FillStyle_NONE)); in
> SdDrawDocument::CreateDefaultCellStyles() but no success. Another idea was
> to read the cell background in GetTextEditBackgroundColor() at if( pTable )
> - but no idea how to.

I'm afraid it's not quite correct.
1. The cell background is read there - the 'GetDraftFillColor(pTable->GetActiveCellItemSet(), aBackground )' call is the first there.
2. The table background (in the call to 'GetDraftFillColor(pText->GetMergedItemSet(), aBackground)') is empty there; but then, its parent item set is queried - and that gives the default Tango Sky Blue.

The option is to not read the parent in the latter call - maybe we need to add a boolean to GetDraftFillColor, to call 'rSet.Get' there with a respective flag. Then, the result from querying the table background would be false, and it will continue correctly...
Comment 17 Heiko Tietze 2023-08-24 10:33:39 UTC
(In reply to Mike Kaganski from comment #16)
> The option is to not read the parent in the latter call...
The parent of a cell is AFAICS the table itself, which has falsely set the solid fill attribute (setting this to None could be a solution). And using a dedicated background / style for cells does not work either - the GetDraftFillColor() function needs to read the TableStyleSettings. Which, however, seems not to know anything about colors. Maybe TableDesignStyle::getCellStyleNameMap() background...
Comment 18 Mike Kaganski 2023-08-24 10:57:03 UTC
(In reply to Heiko Tietze from comment #17)

1. We are not talking about parent of the cell. The cell is read in the 'GetDraftFillColor(pTable->GetActiveCellItemSet(), aBackground )' call, and it returns false. The problematic is the call to 'GetDraftFillColor(pText->GetMergedItemSet(), aBackground)', where the 'pText' is the table; and let me repeat again: inside the call, in some *nested* calls, the merged item set of the pText itself does *not* define a solid color; but *its* parent does.

2. We are not also talking about parent of the cell *or* table, but about the parent of the *respective item sets*, which is a bit different.
Comment 19 Maxim Monastirsky 2023-08-24 12:02:07 UTC
(In reply to Mike Kaganski from comment #16)
> The option is to not read the parent in the latter call - maybe we need to
> add a boolean to GetDraftFillColor, to call 'rSet.Get' there with a
> respective flag.
From my tests it appears that the table-level fill setting is never visualized (or exported to ODF) even when set as DF. If that's the case, I would say we should ignore the table-level filling altogether, not just its parent style.
Comment 20 Mike Kaganski 2023-08-24 12:13:39 UTC
(In reply to Maxim Monastirsky from comment #19)

Hmm, of course that would be a reasonable workaround. But what that property is for, then?

In Writer, there is a drop-down on the Background tab for table (why not Area btw?), allowing to set, if the setting is for table, row, or for cell.

So there's an inconsistency. What does ODF has to say here? Is table background has any meaning for Impress?
Comment 21 Regina Henschel 2023-08-24 13:42:47 UTC
(In reply to Mike Kaganski from comment #20)
> (In reply to Maxim Monastirsky from comment #19)
> 
> Hmm, of course that would be a reasonable workaround. But what that property
> is for, then?
> 
> In Writer, there is a drop-down on the Background tab for table (why not
> Area btw?), allowing to set, if the setting is for table, row, or for cell.

In Writer, a table is on same level as paragraphs. In Writer a table is not a
graphic object. In Draw/Writer a table is child element of a <draw:frame> element.

> 
> So there's an inconsistency. What does ODF has to say here? Is table
> background has any meaning for Impress?

The table in Draw/Impress is a <draw:frame><table:table>...<table:table></draw:frame>.

The <draw:frame> element has a style which has a <style:graphic-properties> child element. That means, that it has all of the fill and stroke properties. If a setting is missing in the frame style, the setting of the <style:default-style> element of style:family="graphic" is used. That is the place were the properties of the "Default Drawing Style" of the UI are stored.

So yes, there exists always a fill property for the shape object in addition to the fill properties of the table.


The fill of a table cell can have different origins:

The <table:table> element has a style which has a <style:table-properties> child element. Table rows have the <style:table-row-properties> and table cells have <style:table-cell-properties>. All these have fill with color and bitmap fill in ODF 1.3. That will be extended to all kind of filling in ODF 1.4. Such styles can be in <office:automatic-styles> (direct formatting) or in <office:styles> (user defined styles).

A table can in addition have a reference to a <table:table-template> element and rules, which parts of the template to use. A table template is a set of references to table cell styles. They are use for a style as long as no style is set in the individual table.

The fill of the <draw:frame> parent element is behind the table and should be visible if the table fill is "transparent".

Only a small part of the possibilities of ODF is actually implemented in LibreOffice.
Comment 22 Mike Kaganski 2023-08-24 13:55:44 UTC
(In reply to Regina Henschel from comment #21)

Thank you very much!

So summing up, the proper fix would be to implement using the table properties, and set the 'none' as the table background. But until it isn't, Maxim's idea is the best, taking into account pText->GetMergedItemSet() *only* when !pTable.
Comment 23 Regina Henschel 2023-08-24 15:51:30 UTC
Please keep in mind, that white text on white background is not only a problem for table-shapes, but for text in notes, thumbnails in notes and handouts, and for data tables in charts as well.
Comment 24 Regina Henschel 2023-09-08 12:00:34 UTC
Created attachment 189434 [details]
Wrong automatic font color in text box in group in docx

And just another example of wrong automatic color. The shape inside the group has wrong font color, the same shape outside the group has correct color.

Please revert/change the fix of bug 156182 until all these here mentioned problems are solved.
Comment 25 Heiko Tietze 2023-09-11 05:28:18 UTC
(In reply to Regina Henschel from comment #24)
> The shape inside the group has wrong font color...
Both fonts are black on some bright background color.
Comment 26 Rafael Lima 2023-09-11 11:16:25 UTC
(In reply to Heiko Tietze from comment #25)
> Both fonts are black on some bright background color.

I tested the file provided by Regina using LO 24.2+ and the one "inside group" has white font over a light background, whereas the other "outside group" has black font over a light background (which is expected).

Right-clicking the group and then Ungrouping it will cause the font color to change to black.

I feel most users will perceive this as a bug.
Comment 27 Xisco Faulí 2023-10-04 09:53:26 UTC
Same issue reproduced with attachment 97012 [details] from bug 77117
Comment 28 Xisco Faulí 2023-10-04 10:05:35 UTC
(In reply to Regina Henschel from comment #24)
> Created attachment 189434 [details]
> Wrong automatic font color in text box in group in docx
> 
> And just another example of wrong automatic color. The shape inside the
> group has wrong font color, the same shape outside the group has correct
> color.
> 
> Please revert/change the fix of bug 156182 until all these here mentioned
> problems are solved.

Done in https://gerrit.libreoffice.org/c/core/+/157537
Comment 29 Heiko Tietze 2023-10-04 16:57:22 UTC
My proposal changes the default drawing style for tables, see https://gerrit.libreoffice.org/c/core/+/157568.
Comment 30 Commit Notification 2023-10-05 07:36:34 UTC
Heiko Tietze committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/86eb7ad2b4488dcd29c21ae3fc525056b681e199

Resolves tdf#156685 - "Object without fill" style for tables

It will be available in 24.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 31 Heiko Tietze 2023-10-06 07:37:20 UTC
*** Bug 142799 has been marked as a duplicate of this bug. ***
Comment 32 Heiko Tietze 2023-10-06 07:44:19 UTC
(In reply to Regina Henschel from comment #5)
> Not only text in tables is affected by the new threshold, but presentation
> notes as well.

bug 157630
Comment 33 Heiko Tietze 2023-10-06 07:48:49 UTC
(In reply to Regina Henschel from comment #24)
> And just another example of wrong automatic color. The shape inside the
> group has wrong font color, the same shape outside the group has correct
> color.

bug 157631
Comment 34 Eyal Rozenberg 2023-10-06 08:16:46 UTC
I would like to test the fix, but I didn't quite catch what the new expected behavior should be. That is, what is the new logic for determining the "automatic" font color?
Comment 35 Xisco Faulí 2023-10-10 09:32:10 UTC
attachment 146336 [details] from bug 121189 is also affected by this issue
Comment 36 Commit Notification 2023-10-16 13:36:11 UTC
Heiko Tietze committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/8ff7d75c1929876743b01d9651ea30928673433f

Revert "Resolves tdf#156685 - "Object without fill" style for tables"

It will be available in 24.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 37 Heiko Tietze 2023-10-16 13:37:20 UTC
Resolved with https://gerrit.libreoffice.org/c/core/+/158024 (new contrast applies to all colors but Light Blue 2 aka Tango Sky, which is the default for object background).
Comment 38 Commit Notification 2023-10-20 23:04:40 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/fc1ba4d17fd71656205697a6cf19432037e4a9d6

tdf#156182, tdf#156685: vcl_pdfexport: Add unittest

It will be available in 24.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.