Created attachment 189277 [details] empty file with color theme Open attached file. It has a custom color theme. Insert a shape and open its area fill dialog. Select the palette 'Theme Colors'. Set the fill to "Light 1". Insert a second shape and set its area fill to "Accent 1". Save the file. Open the file in an editor and inspect the style of the shapes. You will find loext:theme-type="dark2" and loext:theme-type="accent2". The colors in draw:fill-color are correct. Info: The theme definition itself is in <office:master-styles> - <style:master-page> - <loext:theme> - <loext:theme-colors>
On pc Debian x86-64 with master sources updated today, I could reproduce this. When selecting Light1, I get this bt: #0 PaletteManager::GetThemeAndEffectIndex(unsigned short, unsigned short&, unsigned short&) (nItemId=2, rThemeIndex=@0x7ffe9d172eae: 0, rEffectIndex=@0x7ffe9d172eac: 32766) at svx/source/tbxctrls/PaletteManager.cxx:165 #1 0x00007f1922b4e2d9 in SvxColorTabPage::SelectValSetHdl_Impl(ValueSet*) (this=0x56115d44fb90, pValSet=0x56115d732170) at cui/source/tabpages/tpcolor.cxx:511 #2 0x00007f1922b4cd7d in SvxColorTabPage::LinkStubSelectValSetHdl_Impl(void*, ValueSet*) (instance=0x56115d44fb90, data=0x56115d732170) at cui/source/tabpages/tpcolor.cxx:492 With this patch, it seems to work: diff --git a/svx/source/tbxctrls/PaletteManager.cxx b/svx/source/tbxctrls/PaletteManager.cxx index 9b8f144f66b0..77cbab82207d 100644 --- a/svx/source/tbxctrls/PaletteManager.cxx +++ b/svx/source/tbxctrls/PaletteManager.cxx @@ -161,7 +161,10 @@ bool PaletteManager::IsThemePaletteSelected() const bool PaletteManager::GetThemeAndEffectIndex(sal_uInt16 nItemId, sal_uInt16& rThemeIndex, sal_uInt16& rEffectIndex) { - // Each column is the same color with different effects. + // tdf#157034, nItemId begins with 1 but list of themes begin with 0 + // so decrement nItemId + --nItemId; + // Each column is the same color with different effects. rThemeIndex = nItemId % 12; rEffectIndex = nItemId / 12; Regina: does it seem to you?
(In reply to Julien Nabet from comment #1) > With this patch, it seems to work: > diff --git a/svx/source/tbxctrls/PaletteManager.cxx > b/svx/source/tbxctrls/PaletteManager.cxx > index 9b8f144f66b0..77cbab82207d 100644 > --- a/svx/source/tbxctrls/PaletteManager.cxx > +++ b/svx/source/tbxctrls/PaletteManager.cxx > @@ -161,7 +161,10 @@ bool PaletteManager::IsThemePaletteSelected() const > > bool PaletteManager::GetThemeAndEffectIndex(sal_uInt16 nItemId, sal_uInt16& > rThemeIndex, sal_uInt16& rEffectIndex) > { > - // Each column is the same color with different effects. > + // tdf#157034, nItemId begins with 1 but list of themes begin with 0 > + // so decrement nItemId > + --nItemId; > + // Each column is the same color with different effects. > rThemeIndex = nItemId % 12; > > rEffectIndex = nItemId / 12; > > Regina: does it seem to you? Yes, that looks correct. Do you want to write the fix? If yes, please set Tomaž Vajngerl for review. Do you have an idea for a unit test? Likely something with dispatcher. But without further research I couldn't write a test. BTW, the error is visible in export to pptx as well.
(In reply to Regina Henschel from comment #2) > (In reply to Julien Nabet from comment #1) > > > With this patch, it seems to work: > > ... > > Regina: does it seem to you? > > Yes, that looks correct. Do you want to write the fix? If yes, please set > Tomaž Vajngerl for review. Done here https://gerrit.libreoffice.org/c/core/+/156350 > > Do you have an idea for a unit test? Likely something with dispatcher. But > without further research I couldn't write a test. > I don't know much about unit tests, I'm less interested in these (even if I know it's useful to detect regressions early). > ...
I abandoned the patch since the unit test failed. Unassign myself.
Julien Nabet committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/85aa230aebfe383eecdb4fc6704d093db8a18402 tdf#157034: FILESAVE loext:theme-type is off by one 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.
Thank you Regina for the review! I cherry-picked the patch for 7.6 branch here: https://gerrit.libreoffice.org/c/core/+/156389 Let's put this one to FIXED then.
Julien Nabet committed a patch related to this issue. It has been pushed to "libreoffice-7-6": https://git.libreoffice.org/core/commit/a70b613e6b5d95362f34f57df5cda22a74cb670e tdf#157034: FILESAVE loext:theme-type is off by one It will be available in 7.6.2. 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.