Bug 135121 - UI: Font color toolbar icon looks off after setting it to automatic
Summary: UI: Font color toolbar icon looks off after setting it to automatic
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
7.1.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Caolán McNamara
URL:
Whiteboard: target:7.1.0 target:7.0.1
Keywords: bibisectRequest, regression
: 135122 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-07-25 07:26 UTC by Telesto
Modified: 2020-08-06 10:35 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
Missing color (12.21 KB, image/png)
2020-08-01 10:37 UTC, Rizal Muttaqin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Telesto 2020-07-25 07:26:42 UTC
Description:
UI: Font color toolbar icon looks off after setting it to automatic

Steps to Reproduce:
1. Open writer
2. font color pull down
3. Select automatic. Red line shrinks in size

Actual Results:
Red line shrinks in size

Expected Results:
Same look as always. And maybe not red?


Reproducible: Always


User Profile Reset: No



Additional Info:
Version: 7.0.0.2
Build ID: c01aa64b6c3d89ebe5fe69c28c7adb24eb85249c
CPU threads: 4; OS: Mac OS X 10.12.6; UI render: default; VCL: osx
Locale: nl-NL (nl_NL.UTF-8); UI: en-US
Calc: threaded
Comment 1 Telesto 2020-07-25 07:58:43 UTC
Can't be bibisected on Windows.. large range
Comment 2 Julien Nabet 2020-07-25 09:51:22 UTC
On pc Debian x86-64 with master sources updated today, I gave it a try.
I don't know what red line you're talking about.

However, I noticed these logs:
warn:legacy.osl:13838:13838:svl/source/items/custritm.cxx:70: CntUnencodedStringItem::PutValue(): Wrong type
warn:sfx:13838:13838:sfx2/source/appl/appuno.cxx:310: Property not convertible: Color
Comment 3 Julien Nabet 2020-07-25 09:53:48 UTC
More precisely, it seems there are 2 bars in the dropdown, the second one overlaps a bit the first one and the first one is a bit shorter than second one.
At the beginning first one is blank, the second is red.
If I select automatic, the first one becomes a rainbow and the second one is blank.
Comment 4 Julien Nabet 2020-07-25 10:11:47 UTC
Noel: thought you might be interested in this one.
Comment 5 Maxim Monastirsky 2020-07-26 07:18:58 UTC
Basically I see 2 problems here:

1) The color button is bigger than the other buttons on the toolbar, causing the color stripe to not align well with the stripe from the base icon (red or rainbow, depending on the icon theme), so some pixels of the icon's stripe are always visible. This seems to be a result of:

commit 3c17bb91f29d431f99a742bd14ffc9612f25a7af
Author: Caolán McNamara <caolanm@redhat.com>
Date:   Fri Jun 19 15:06:23 2020 +0100

    tdf#134084 use the desired image size, not the current image size
    
    when updating the color bar
    
    Change-Id: I6bf76a38adc034a25c0c31833c14f2b6afe3ec6d

This commit hardcoded height of 26 for large size, but some icon themes actually have height of 24.

2) The stripe for automatic color is transparent (with just a black border), revealing what's in the base icon underneath it. That's a red or rainbow, depending on the icon theme. Possible solutions are: (a) Not have any stripe in the base icon, see Bug 134928 comment 9. (b) Use an actual fill color instead of transparent to indicate the automatic color. This actually used to be the case in the past (at least in LO 5.3). This isn't a recent regression, but still might be useful to bibisect.
Comment 6 Maxim Monastirsky 2020-07-26 07:30:08 UTC
*** Bug 135122 has been marked as a duplicate of this bug. ***
Comment 7 Commit Notification 2020-07-28 10:12:18 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

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

tdf#135121 ImageType::Size26 isn't always 26x26px

It will be available in 7.1.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 8 Commit Notification 2020-07-28 10:13:29 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/9282871ab05721c59f7946879a5f28f2e8af96a1

Related: tdf#135121 update widebuttons on icon-size change too

It will be available in 7.1.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 9 Commit Notification 2020-07-28 10:13:43 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-0":

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

tdf#135121 ImageType::Size26 isn't always 26x26px

It will be available in 7.0.1.

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 10 Caolán McNamara 2020-07-28 11:17:23 UTC
The above should address a, and for b we could do https://gerrit.libreoffice.org/c/core/+/99589 and exclude painting that part of the image in the first place, so we retain the original device wallpaper/background if the fill color is transparent
Comment 11 Commit Notification 2020-07-28 18:30:17 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

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

tdf#135121 don't paint the image over the rect that will contain the color

It will be available in 7.1.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 12 Caolán McNamara 2020-07-28 18:31:45 UTC
that seems to work, fixed in master, backport to 7-0 in gerrit
Comment 13 Rizal Muttaqin 2020-08-01 10:37:46 UTC
Created attachment 163844 [details]
Missing color

I have tried change font color to red and set yellow highlight but now the color i s gone. Is this missing color box below related to this bug report?
Comment 14 Rizal Muttaqin 2020-08-01 10:38:28 UTC
(In reply to Rizal Muttaqin from comment #13)
> Created attachment 163844 [details]
> Missing color
> 
> I have tried change font color to red and set yellow highlight but now the
> color i s gone. Is this missing color box below related to this bug report?

Version: 7.1.0.0.alpha0+ (x64)
Build ID: <buildversion>
CPU threads: 4; OS: Windows 10.0 Build 19041; UI render: Skia/Vulkan; VCL: win
Locale: id-ID (id_ID); UI: en-US
Calc: threaded
Comment 15 Caolán McNamara 2020-08-01 14:17:10 UTC
I don't know, have you tried reverting it locally to find out? Seems to work ok for me in Linux/gen.
Comment 16 Telesto 2020-08-01 17:51:19 UTC
(In reply to Rizal Muttaqin from comment #14)
> (In reply to Rizal Muttaqin from comment #13)
> > Created attachment 163844 [details]
> > Missing color
> > 
> > I have tried change font color to red and set yellow highlight but now the
> > color i s gone. Is this missing color box below related to this bug report?

Same observation
Version: 7.1.0.0.alpha0+ (x64)
Build ID: <buildversion>
CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: Skia/Raster; VCL: win
Locale: nl-NL (nl_NL); GI: nl-NL
Calc: CL

It's surely in the same time span as the commit
Comment 17 Rizal Muttaqin 2020-08-01 22:30:25 UTC
(In reply to Rizal Muttaqin from comment #14)

> Build ID: <buildversion>

Notice that buildversion did not show up

(In reply to Caolán McNamara from comment #15)

> I don't know, have you tried reverting it locally to find out? Seems to work ok for me in Linux/gen.

I don't build it myself, always using TDF's daily master build
Comment 18 Maxim Monastirsky 2020-08-01 23:29:04 UTC
I do see a problem with gen when selecting "Automatic". The color stripe shows some garbage instead of the toolbar background color, and sometimes it just keeps the previously selected color. Can be more easily seen with large or extra large button size.
Comment 19 Caolán McNamara 2020-08-02 14:06:36 UTC
how about https://gerrit.libreoffice.org/c/core/+/99955 do that give an acceptable result?
Comment 20 Maxim Monastirsky 2020-08-02 15:08:55 UTC
(In reply to Caolán McNamara from comment #19)
> how about https://gerrit.libreoffice.org/c/core/+/99955 do that give an
> acceptable result?
Works nicely, thanks!
Comment 21 Commit Notification 2020-08-02 15:43:56 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/94811eac3bd3077b60029c52d7798bca9b3c877b

tdf#135121 don't fill the rectangle in the transparent case at all

It will be available in 7.1.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 22 Caolán McNamara 2020-08-02 15:48:48 UTC
ok, and merged that into the open 7-0 https://gerrit.libreoffice.org/c/core/+/99549 backport
Comment 23 Commit Notification 2020-08-03 04:37:27 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-0":

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

tdf#135121 don't paint the image over the rect that will contain the color

It will be available in 7.0.1.

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 24 Telesto 2020-08-05 19:12:41 UTC
Reopening again; it's still not solved
Version: 7.1.0.0.alpha0+ (x64)
Build ID: <buildversion>
CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: Skia/Raster; VCL: win
Locale: ru-RU (nl_NL); UI: en-US
Calc: CL

-> Build today

However not sure who to blame here ;-). It's Skia Raster/Vulkan only. Fine with GDI.
Comment 25 Caolán McNamara 2020-08-06 08:31:13 UTC
"Reopening again; it's still not solved ... It's Skia Raster/Vulkan only. Fine with GDI." sounds like a different problem, a skia-specific one. A similar one, but a different one to what was addressed here.
Comment 26 Caolán McNamara 2020-08-06 10:35:58 UTC
lets leave this general case closed, as that seems to be ok, and leave bug #135487 for the skia case