Bug 132591 - The Keep ratio option for the caption frame can't be deactivated
Summary: The Keep ratio option for the caption frame can't be deactivated
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
7.3.0.0.alpha1+
Hardware: All All
: medium normal
Assignee: Caolán McNamara
URL:
Whiteboard: target:7.4.0 target:7.3.1
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2020-05-01 15:14 UTC by Telesto
Modified: 2022-02-20 10:32 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
Example file (13.03 KB, application/vnd.oasis.opendocument.text)
2020-06-17 19:47 UTC, Telesto
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Telesto 2020-05-01 15:14:30 UTC
Description:
The Keep ratio button for the caption frame can't be re-enabled after disabling

Steps to Reproduce:
1. open the attached file
2. Insert Caption
3. Type a name & press OK
4. Format -> Frame and objects -> Properties
5. Type Tab
6. Uncheck "Keep Ratio"
7. Press ok
8. Format -> Frame and objects -> Properties
9. Recheck "Keep Ratio"
10. Press OK
11. Format -> Frame and objects -> Properties
12. Type Tab -> Keep ratio unchecked

Actual Results:
Keep ratio unchecked after checking

Expected Results:
Should stay checked


Reproducible: Always


User Profile Reset: No



Additional Info:
In 
Version: 7.0.0.0.alpha0+ (x64)
Build ID: f845f74afaf087a46c82ee4209e29caca0980b71
CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: default; VCL: win; 
Locale: nl-NL (nl_NL); UI-Language: en-US
Calc: CL

No repro with
Versie: 4.4.7.2 
Build ID: f3153a8b245191196a4b6b9abd1d0da16eead600
Locale: nl_NL
Comment 1 Julien Nabet 2020-05-02 09:31:52 UTC
On pc Debian x86-64 with master sources updated today, when enabling Keep ratio first time, then OK and reopen the dialog, "Keep ratio" is still disabled.

So no need here to enable then disable a first time for me to reproduce this.
Comment 2 Xisco Faulí 2020-06-17 15:42:16 UTC
Please attached the document mentioned in step 1
Comment 3 Telesto 2020-06-17 19:47:00 UTC
Created attachment 162127 [details]
Example file
Comment 4 Telesto 2020-06-17 19:47:26 UTC
Hmm. currently the opposite... can't disable..
Comment 5 Dieter 2020-09-24 10:59:22 UTC
Tested with

Version: 7.0.1.2 (x64)
Build ID: 7cbcfc562f6eb6708b5ff7d7397325de9e764452
CPU threads: 4; OS: Windows 10.0 Build 19041; UI render: Skia/Raster; VCL: win
Locale: de-DE (de_DE); UI: en-GB
Calc: threaded

Step 6: After disabelling "Keep Ratio" height changes from 5,46cm (at least) to 9,18 cm (at least).
Step 7: O.K.

Expected result: Height of shape and caption frame should change
Actual result: height of shape and caption frame doesn't change
Comment 6 Buovjaga 2021-07-26 09:43:06 UTC
No problem either way, please re-test.

NixOS
Version: 7.3.0.0.alpha0+ / LibreOffice Community
Build ID: 67e47070a7580a17804adce812cc2f98bfe7b51f
CPU threads: 16; OS: Linux 5.13; UI render: default; VCL: x11
Locale: fi-FI (fi_FI.UTF-8); UI: en-US
Calc: threaded
Comment 7 QA Administrators 2022-01-23 03:43:00 UTC Comment hidden (obsolete)
Comment 8 Telesto 2022-01-23 08:56:46 UTC
Currently the inverse is happening..

Unchecking Keep Ratio & pressing OK & opening dialog again results in Keep Ratio being checked!
Comment 9 raal 2022-01-23 09:13:09 UTC
(In reply to Telesto from comment #8)
> Currently the inverse is happening..
> 
> Unchecking Keep Ratio & pressing OK & opening dialog again results in Keep
> Ratio being checked!

No repro Version: 7.4.0.0.alpha0+ / LibreOffice Community
Build ID: bd5492275d31f59b1d269205018d1487af52426f
CPU threads: 4; OS: Linux 5.11; UI render: default; VCL: gtk3
Locale: cs-CZ (cs_CZ.UTF-8); UI: en-US
Calc: threaded Jumbo

Version: 6.4.0.0.alpha1+
Build ID: 9bc848cf0d301aa57eabcffa101a1cf87bad6470
CPU threads: 4; OS: Linux 5.11; UI render: default; VCL: x11; 
Locale: cs-CZ (cs_CZ.UTF-8); UI-Language: en-US
Calc: threaded

but I can repro with Version: 7.4.0.0.alpha0+ / LibreOffice Community
Build ID: bd5492275d31f59b1d269205018d1487af52426f
CPU threads: 4; OS: Linux 5.11; UI render: default; VCL: x11
Locale: cs-CZ (cs_CZ.UTF-8); UI: en-US
Calc: threaded Jumbo
Comment 10 Buovjaga 2022-01-23 09:23:20 UTC
Bibisected the change with linux-64-7.3 to https://git.libreoffice.org/core/commit/dd5039ceea319ebfc72a3e032753b22538add12e

Resolves tdf#143633 - Image size wrong after disabling relative image width

Adding Cc: to Heiko Tietze
Comment 11 Julien Nabet 2022-01-23 10:52:26 UTC
(In reply to Buovjaga from comment #10)
> Bibisected the change with linux-64-7.3 to
> https://git.libreoffice.org/core/commit/
> dd5039ceea319ebfc72a3e032753b22538add12e
> 
> Resolves tdf#143633 - Image size wrong after disabling relative image width
> 
> Adding Cc: to Heiko Tietze

The quoted patch has been done in 08/2021, the bugtracker has been created in 2020.
Comment 12 Buovjaga 2022-01-23 11:02:42 UTC
(In reply to Julien Nabet from comment #11)
> (In reply to Buovjaga from comment #10)
> > Bibisected the change with linux-64-7.3 to
> > https://git.libreoffice.org/core/commit/
> > dd5039ceea319ebfc72a3e032753b22538add12e
> > 
> > Resolves tdf#143633 - Image size wrong after disabling relative image width
> > 
> > Adding Cc: to Heiko Tietze
> 
> The quoted patch has been done in 08/2021, the bugtracker has been created
> in 2020.

and the topic of the bug report was changed in comment 8
Comment 13 Heiko Tietze 2022-02-08 14:20:03 UTC
My patch does not even touch any checkbox. It just adds GtkAdjustments to keep the values within a range of -100/+100 resp 0.5/100. 

It's quite confusing when the topic changes. Maybe I don't understand issue.
Comment 14 Buovjaga 2022-02-08 14:34:24 UTC
(In reply to Heiko Tietze from comment #13)
> My patch does not even touch any checkbox. It just adds GtkAdjustments to
> keep the values within a range of -100/+100 resp 0.5/100. 
> 
> It's quite confusing when the topic changes. Maybe I don't understand issue.

Well, I just now checked by reverting your patch and it makes the problem described in comment 8 go away. Please try it.
Comment 15 Julien Nabet 2022-02-08 17:58:14 UTC
(In reply to Heiko Tietze from comment #13)
> My patch does not even touch any checkbox. It just adds GtkAdjustments to
> keep the values within a range of -100/+100 resp 0.5/100. 
> 
> It's quite confusing when the topic changes. Maybe I don't understand issue.

On pc Debian x86-64 with master sources updated today, I could reproduce this without reverting the quoted patch.
If I revert locally the quoted patch, I don't reproduce this.

I must recognize I don't understand why since your patch doesn't include keep-ratio part action, perhaps a side-effect?
Comment 16 Julien Nabet 2022-02-08 18:06:13 UTC
I made other tests.
Just removing adjustement2 part seems to remove the pb.
I'm keeping on to dig.
Comment 17 Julien Nabet 2022-02-08 19:20:20 UTC
(In reply to Julien Nabet from comment #16)
> I made other tests.
> Just removing adjustement2 part seems to remove the pb.
> I'm keeping on to dig.

If I let "adjustment2", I only need to remove "upper" property to remove the pb.
I don't know why for the moment.
Comment 18 Julien Nabet 2022-02-08 20:58:33 UTC
I tried to compare use of m_xWidthED and m_xHeightED and I'm pretty quite sure there are some errors in sw/source/ui/frmdlg/frmpage.cxx
diff --git a/sw/source/ui/frmdlg/frmpage.cxx b/sw/source/ui/frmdlg/frmpage.cxx
index 49999bb82586..22aa31f3ec16 100644
--- a/sw/source/ui/frmdlg/frmpage.cxx
+++ b/sw/source/ui/frmdlg/frmpage.cxx
@@ -1845,7 +1845,7 @@ void SwFramePage::RangeModifyHdl()
         m_xWidthED->set_max(m_xWidthED->NormalizePercent(nTmp), FieldUnit::TWIP);
 
         nTmp = std::min(nHeight * nMaxWidth / std::max(nWidth, SwTwips(1)), nMaxWidth);
-        m_xHeightED->set_max(m_xWidthED->NormalizePercent(nTmp), FieldUnit::TWIP);
+        m_xHeightED->set_max(m_xHeightED->NormalizePercent(nTmp), FieldUnit::TWIP);
     }
     else
     {
@@ -2046,7 +2046,7 @@ IMPL_LINK_NOARG(SwFramePage, AutoWidthClickHdl, weld::Toggleable&, void)
 IMPL_LINK_NOARG(SwFramePage, AutoHeightClickHdl, weld::Toggleable&, void)
 {
     if (!IsInGraficMode())
-        HandleAutoCB(m_xAutoHeightCB->get_active(), *m_xHeightFT, *m_xHeightAutoFT, *m_xWidthED->get());
+        HandleAutoCB(m_xAutoHeightCB->get_active(), *m_xHeightFT, *m_xHeightAutoFT, *m_xHeightED->get());
 }
 
 IMPL_LINK( SwFramePage, ModifyHdl, weld::MetricSpinButton&, rEdit, void )
@@ -2163,7 +2163,7 @@ void SwFramePage::Init(const SfxItemSet& rSet)
         SwFrameSize eSize = rSize.GetHeightSizeType();
         bool bCheck = eSize != SwFrameSize::Fixed;
         m_xAutoHeightCB->set_active(bCheck);
-        HandleAutoCB( bCheck, *m_xHeightFT, *m_xHeightAutoFT, *m_xWidthED->get() );
+        HandleAutoCB( bCheck, *m_xHeightFT, *m_xHeightAutoFT, *m_xHeightED->get() );
         if( eSize == SwFrameSize::Variable )
             m_xHeightED->set_value(m_xHeightED->get_min());


Also, this part seems a bit weird:
   1842     if (aVal.bAutoHeight && (m_sDlgType == "PictureDialog" || m_sDlgType == "ObjectDialog"))
   1843     {
   1844         SwTwips nTmp = std::min(nWidth * nMaxHeight / std::max(nHeight, SwTwips(1)), nMaxHeight);
   1845         m_xWidthED->set_max(m_xWidthED->NormalizePercent(nTmp), FieldUnit::TWIP);
   1846 
   1847         nTmp = std::min(nHeight * nMaxWidth / std::max(nWidth, SwTwips(1)), nMaxWidth);
   1848         m_xHeightED->set_max(m_xHeightED->NormalizePercent(nTmp), FieldUnit::TWIP);
   1849     }

Now I still don't find the link with "upper" property.
=> I give up and uncc myself.
Comment 19 Caolán McNamara 2022-02-09 11:14:44 UTC
sw/source/ui/frmdlg/frmpage.cxx line 2254 has a fascinating...

if (rSize.GetWidthPercent() == SwFormatFrameSize::SYNCED ||
    rSize.GetHeightPercent() == SwFormatFrameSize::SYNCED)
{
    m_xFixedRatioCB->set_active(true);
}

where sw/inc/fmtfsize.hxx: enum PercentFlags { SYNCED = 0xff };

which gives some indication how this change could be related to this odd effect. let me have a look at this
Comment 20 Caolán McNamara 2022-02-09 14:00:20 UTC
I think this has not worked for a long time, but during the window of time between eb807e785b6acda1529b2f58ada2c1e84b38f7f5 and dd5039ceea319ebfc72a3e032753b22538add12e there was no adjustment set and a side effect of that bug was that the height/width were always seen as modified so the tabpage was considered "changed". I think https://gerrit.libreoffice.org/c/core/+/129730 is a suitable way to fix this
Comment 21 Commit Notification 2022-02-09 19:58:43 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/5cefbd837a9e41102270d8c60010e3496b76f0a0

Related: tdf#132591 connect adjustments to the correct spinbuttons

It will be available in 7.4.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 Commit Notification 2022-02-09 19:59:52 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

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

Resolves: tdf#132591 set property if Fixed Ratio checkbox was toggled

It will be available in 7.4.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 23 Caolán McNamara 2022-02-09 20:12:00 UTC
ratio button state seems to be now get set/unset as expected in trunk, backports to 7-3 in gerrit
Comment 24 Commit Notification 2022-02-10 02:51:23 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

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

Related: tdf#132591 connect adjustments to the correct spinbuttons

It will be available in 7.3.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 25 Commit Notification 2022-02-10 02:52:32 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

https://git.libreoffice.org/core/commit/27b03993c49b3c8a3bafa736db9ca25e834a2195

Resolves: tdf#132591 set property if Fixed Ratio checkbox was toggled

It will be available in 7.3.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.