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