Re: broken radio/checkboxes in LibreOffice 6.1 (was: Re: UI regressions in "gen" plugin with LibreOffice 6.1)
Hi,
On Sun, Aug 12, 2018 at 04:58:52PM +0300, Maxim Monastirsky wrote:
> On Sun, 2018-08-12 at 14:43 +0200, Rene Engelhard wrote:
> > so vcl/source/app/IconThemeSelector.cxx
> >
> > > installedThemes)
> > > {
> > > if (!installedThemes.empty()) {
> > > return installedThemes.front().GetThemeId();
> > > }
> > > else {
> > > return FALLBACK_ICON_THEME_ID;
> > > }
> > > }
> >
> >
> > $ git grep FALLBACK_ICON_THEME_ID
> > source/app/IconThemeSelector.cxx:/*static*/ const OUStringLiteral
> > IconThemeSelector::FALLBACK_ICON_THEME_ID("tango");
> >
> > ^
> > ^
> >
>
> IconThemeSelector::ReturnFallback has a parameter const
> std::vector& installedThemes. The code which calls this
> method, passes there a reference to IconThemeScanner::mFoundIconThemes,
> which dynamically filled with the themes found in the installation
> folder. Now, it returns FALLBACK_ICON_THEME_ID (currently set to tango)
> only if installedThemes.empty(), meaning that we couldn't find any icon
> theme in the installation folder (including tango!). So normally, when
> there is at least one theme installed, this "return
> FALLBACK_ICON_THEME_ID" will never happen.
Ah, I see, didn't look that deep I have to admit. And because one other
theme is installed (tango) it took that one and it broke.
Thanks for the explanation.
(As said: libreoffice-common now Depends: libreoffice-style-colibre,
libreoffice-style-tango so it should now work in all "default" cases.
And -gnome/-kde Depends: libreoffice-style-elementary /
libreoffice-style-breeze for their iconsets)
Regards,
Rene
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: broken radio/checkboxes in LibreOffice 6.1 (was: Re: UI regressions in "gen" plugin with LibreOffice 6.1)
On Sun, 2018-08-12 at 14:43 +0200, Rene Engelhard wrote:
> so vcl/source/app/IconThemeSelector.cxx
>
> > installedThemes)
> > {
> > if (!installedThemes.empty()) {
> > return installedThemes.front().GetThemeId();
> > }
> > else {
> > return FALLBACK_ICON_THEME_ID;
> > }
> > }
>
>
> $ git grep FALLBACK_ICON_THEME_ID
> source/app/IconThemeSelector.cxx:/*static*/ const OUStringLiteral
> IconThemeSelector::FALLBACK_ICON_THEME_ID("tango");
>
> ^
> ^
>
IconThemeSelector::ReturnFallback has a parameter const
std::vector& installedThemes. The code which calls this
method, passes there a reference to IconThemeScanner::mFoundIconThemes,
which dynamically filled with the themes found in the installation
folder. Now, it returns FALLBACK_ICON_THEME_ID (currently set to tango)
only if installedThemes.empty(), meaning that we couldn't find any icon
theme in the installation folder (including tango!). So normally, when
there is at least one theme installed, this "return
FALLBACK_ICON_THEME_ID" will never happen.
Maxim
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: broken radio/checkboxes in LibreOffice 6.1 (was: Re: UI regressions in "gen" plugin with LibreOffice 6.1)
Hi,
On Sun, Aug 12, 2018 at 12:09:42PM +0300, Maxim Monastirsky wrote:
> On Sun, 2018-08-12 at 09:52 +0200, Rene Engelhard wrote:
> > > I think you should use colibre *instead* of tango. My understanding
> > > is
> >
> > No, that looks too much Windows'ishy to me.
>
> Ah, OK. In this case it might make sense to do an upstream patch to
> copy these icons to tango, as we already did for a lot of other icons
> that were copied to tango from galaxy (see tdf#118123).
>
> > > that colibre is at least as complete as tango, and is actively
> > > maintained unlike tango. And tango isn't even part of the fallback
> >
> > That is not 100% true, see my paste.
> >
> > For the desktop detection icon detection it is.
>
> The desktop detection code you cited is about defining the *preferred*
> icon theme, not about "fallback" - which means what to do when the
> requested theme isn't found. The fallback for a complete icon theme (as
> opposed to an individual icon) is done like this:
>
> /*static*/ OUString
> IconThemeSelector::ReturnFallback(const std::vector&
$ git grep ReturnFallback
source/app/IconThemeSelector.cxx:return ReturnFallback(installedThemes);
source/app/IconThemeSelector.cxx:return ReturnFallback(installedThemes);
source/app/IconThemeSelector.cxx:IconThemeSelector::ReturnFallback(const
std::vector& installedThemes)
so vcl/source/app/IconThemeSelector.cxx
> installedThemes)
> {
> if (!installedThemes.empty()) {
> return installedThemes.front().GetThemeId();
> }
> else {
> return FALLBACK_ICON_THEME_ID;
> }
> }
$ git grep FALLBACK_ICON_THEME_ID
qa/cppunit/app/test_IconThemeSelector.cxx:
OUString(vcl::IconThemeSelector::FALLBACK_ICON_THEME_ID), selected);
source/app/IconThemeSelector.cxx:/*static*/ const OUStringLiteral
IconThemeSelector::FALLBACK_ICON_THEME_ID("tango");
^
^
source/app/IconThemeSelector.cxx:r = FALLBACK_ICON_THEME_ID;
source/app/IconThemeSelector.cxx:return FALLBACK_ICON_THEME_ID;
So what is this then? Count me confused. :)
Regards,
Rene
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: broken radio/checkboxes in LibreOffice 6.1 (was: Re: UI regressions in "gen" plugin with LibreOffice 6.1)
Tango actually got the remaining ICP s, or should at least. I will check it
when back next Thursday.
On 12 August 2018 11:09:42 CEST, Maxim Monastirsky wrote:
>On Sun, 2018-08-12 at 09:52 +0200, Rene Engelhard wrote:
>> > I think you should use colibre *instead* of tango. My understanding
>> > is
>>
>> No, that looks too much Windows'ishy to me.
>
>Ah, OK. In this case it might make sense to do an upstream patch to
>copy these icons to tango, as we already did for a lot of other icons
>that were copied to tango from galaxy (see tdf#118123).
>
>> > that colibre is at least as complete as tango, and is actively
>> > maintained unlike tango. And tango isn't even part of the fallback
>>
>> That is not 100% true, see my paste.
>>
>> For the desktop detection icon detection it is.
>
>The desktop detection code you cited is about defining the *preferred*
>icon theme, not about "fallback" - which means what to do when the
>requested theme isn't found. The fallback for a complete icon theme (as
>opposed to an individual icon) is done like this:
>
>/*static*/ OUString
>IconThemeSelector::ReturnFallback(const std::vector&
>installedThemes)
>{
>if (!installedThemes.empty()) {
>return installedThemes.front().GetThemeId();
>}
>else {
>return FALLBACK_ICON_THEME_ID;
>}
>}
>
>... which means that it will return the first found theme, which might
>or might not be tango. Imagine a case of a user running gnome or kde
>(which have other defined themes in
>IconThemeSelector::GetIconThemeForDesktopEnvironment), while having
>only tango and colibre. It will get through the above code, and might
>pick colibre as well, which is not what you want.
>
>Maxim
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: broken radio/checkboxes in LibreOffice 6.1 (was: Re: UI regressions in "gen" plugin with LibreOffice 6.1)
On Sun, 2018-08-12 at 09:52 +0200, Rene Engelhard wrote:
> > I think you should use colibre *instead* of tango. My understanding
> > is
>
> No, that looks too much Windows'ishy to me.
Ah, OK. In this case it might make sense to do an upstream patch to
copy these icons to tango, as we already did for a lot of other icons
that were copied to tango from galaxy (see tdf#118123).
> > that colibre is at least as complete as tango, and is actively
> > maintained unlike tango. And tango isn't even part of the fallback
>
> That is not 100% true, see my paste.
>
> For the desktop detection icon detection it is.
The desktop detection code you cited is about defining the *preferred*
icon theme, not about "fallback" - which means what to do when the
requested theme isn't found. The fallback for a complete icon theme (as
opposed to an individual icon) is done like this:
/*static*/ OUString
IconThemeSelector::ReturnFallback(const std::vector&
installedThemes)
{
if (!installedThemes.empty()) {
return installedThemes.front().GetThemeId();
}
else {
return FALLBACK_ICON_THEME_ID;
}
}
... which means that it will return the first found theme, which might
or might not be tango. Imagine a case of a user running gnome or kde
(which have other defined themes in
IconThemeSelector::GetIconThemeForDesktopEnvironment), while having
only tango and colibre. It will get through the above code, and might
pick colibre as well, which is not what you want.
Maxim
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: broken radio/checkboxes in LibreOffice 6.1 (was: Re: UI regressions in "gen" plugin with LibreOffice 6.1)
On Sun, Aug 12, 2018 at 10:44:21AM +0300, Maxim Monastirsky wrote: > only karasa_jaga has these images, but it isn't part of the fallback > mechanism as you saw. ... and this one isn't packaged by me anyway. Didn't see a need and didn't want to have months of delay for approving the new package. (Any style has a extra package here.) Maybe I enable it for 6.2 given I need NEW anyway because of -qt5... Regards, Rene ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: broken radio/checkboxes in LibreOffice 6.1 (was: Re: UI regressions in "gen" plugin with LibreOffice 6.1)
Hi, On Sun, Aug 12, 2018 at 10:44:21AM +0300, Maxim Monastirsky wrote: > On Sun, 2018-08-12 at 08:46 +0200, Rene Engelhard wrote: > > Will tango work there, > > No, as it doesn't have the radio/checkboxes images. Besides colibre, sigh, OK, so I did https://salsa.debian.org/libreoffice-team/libreoffice/libreoffice/commit/3c1aee743e2de32b636ce906145ce588d9f285af > only karasa_jaga has these images, but it isn't part of the fallback > mechanism as you saw. > > > or do I really need to add a extra > > Depends: on colibre? > I think you should use colibre *instead* of tango. My understanding is No, that looks too much Windows'ishy to me. > that colibre is at least as complete as tango, and is actively > maintained unlike tango. And tango isn't even part of the fallback That is not 100% true, see my paste. For the desktop detection icon detection it is. For the "other" fallback you are right... Regards, Rene ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: broken radio/checkboxes in LibreOffice 6.1 (was: Re: UI regressions in "gen" plugin with LibreOffice 6.1)
On Sun, 2018-08-12 at 08:46 +0200, Rene Engelhard wrote: > Will tango work there, No, as it doesn't have the radio/checkboxes images. Besides colibre, only karasa_jaga has these images, but it isn't part of the fallback mechanism as you saw. > or do I really need to add a extra > Depends: on colibre? I think you should use colibre *instead* of tango. My understanding is that colibre is at least as complete as tango, and is actively maintained unlike tango. And tango isn't even part of the fallback mechanism currently, so depending on it is pointless (unless we change the fallback list again). Maxim ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: broken radio/checkboxes in LibreOffice 6.1 (was: Re: UI regressions in "gen" plugin with LibreOffice 6.1)
On Sun, Aug 12, 2018 at 08:56:23AM +0200, Heiko Tietze wrote: >Fallback was reworked. And Tango should also be complete in 6.1 after >copying the remaining icons from Galaxy So we should update the second case also to tango? Build with that change for testing just running. Regards, Rene ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: broken radio/checkboxes in LibreOffice 6.1 (was: Re: UI regressions in "gen" plugin with LibreOffice 6.1)
Don't the icon themes have weird links files that allow each icon theme to copy icons from other themes? Perhaps just a matter of updating those files so those other themes get the necessary checkbox, etc icons? ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: broken radio/checkboxes in LibreOffice 6.1 (was: Re: UI regressions in "gen" plugin with LibreOffice 6.1)
Fallback was reworked. And Tango should also be complete in 6.1 after copying the remaining icons from Galaxy -- Sent from my Android device with K-9 Mail. Please excuse my brevity.___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: broken radio/checkboxes in LibreOffice 6.1 (was: Re: UI regressions in "gen" plugin with LibreOffice 6.1)
Hi again,
On Sun, Aug 12, 2018 at 08:25:19AM +0200, Rene Engelhard wrote:
> Or does it use an other fallback=
Ah, vcl/source/image/ImplImageTree.cxx:
OUString ImplImageTree::fallbackStyle(const OUString& rsStyle)
{
OUString sResult;
if (rsStyle == "colibre" || rsStyle == "helpimg")
sResult = "";
else if (rsStyle == "sifr" || rsStyle == "breeze_dark")
sResult = "breeze";
else if (rsStyle == "sifr_dark" )
sResult = "breeze_dark";
else
sResult = "colibre";
return sResult;
}
Looks like this wasn't updated when the d efault changed (again)
during 6.1 development?
Will tango work there, too or do I really need to add a extra
Depends: on colibre?
Regards,
Rene
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: broken radio/checkboxes in LibreOffice 6.1 (was: Re: UI regressions in "gen" plugin with LibreOffice 6.1)
Hi,
On Sun, Aug 12, 2018 at 12:41:02AM +0300, Maxim Monastirsky wrote:
> The non-native code path inside vcl uses images from the icon theme to
> draw radio/checkboxes. The problem is that not all icon themes have all
> needed images, so e.g. Elementary does not have them but Colibre does.
Eh, ok.
> In practice, all you need is to always have Colibre installed, as it's
:(
> defined as the last fallback.
vcl/source/app/IconThemeSelector.cxx:
/*static*/ const OUStringLiteral
IconThemeSelector::FALLBACK_ICON_THEME_ID("tango");
[...]
/*static*/ OUString
IconThemeSelector::GetIconThemeForDesktopEnvironment(const OUString&
desktopEnvironment)
{
OUString r;
#ifdef _WIN32
r = "colibre";
(void)desktopEnvironment;
#else
if ( desktopEnvironment.equalsIgnoreAsciiCase("kde4") ||
desktopEnvironment.equalsIgnoreAsciiCase("kde5") ) {
r = "breeze";
} else
if ( desktopEnvironment.equalsIgnoreAsciiCase("macosx") ) {
#if MPL_HAVE_SUBSET
r = "tango";
#else
r = "breeze";
#endif
} else
if ( desktopEnvironment.equalsIgnoreAsciiCase("gnome") ||
desktopEnvironment.equalsIgnoreAsciiCase("mate") ||
desktopEnvironment.equalsIgnoreAsciiCase("unity") ) {
r = "elementary";
} else
{
r = FALLBACK_ICON_THEME_ID;
}
#endif
return r;
}
Or does it use an other fallback=
> Hope that will solve the problem.
It does...
But my packages (because of the above) already do
Package: libreoffice-common
Architecture: all
Depends: libreoffice-style-tango, ure, ${numbertext-data-recommends},
${misc:Depends}
Suggests: libreoffice-style,
[...]
So I really need to add a hard colibre Depends here, too? :(
Regards,
Rene
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: broken radio/checkboxes in LibreOffice 6.1 (was: Re: UI regressions in "gen" plugin with LibreOffice 6.1)
Hi Rene, The non-native code path inside vcl uses images from the icon theme to draw radio/checkboxes. The problem is that not all icon themes have all needed images, so e.g. Elementary does not have them but Colibre does. In practice, all you need is to always have Colibre installed, as it's defined as the last fallback. Hope that will solve the problem. Maxim ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
broken radio/checkboxes in LibreOffice 6.1 (was: Re: UI regressions in "gen" plugin with LibreOffice 6.1)
On Tue, Aug 07, 2018 at 12:47:04PM +0200, Heiko Tietze wrote: > No problem here. But haphazardly I could imagine it's the expander icon (see > screenshot) that is not painted for some reasons ending up in a zero/one > pixel height for the first line. This is my setup: How would I debug this? gtk2 works, OpenGL doesn't matter as it's off afaics. A new incarnation of this is http://bugs.debian.org/905819. That screenshot looks GTKish for the "normal" UI so it seems drawing those buttons by LO is broken? Regards, Rene ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
