Created attachment 173542 [details] Test document to show the problem Undo and Redo (indicator arrows as well as actions) are not properly handled for checking and unchecking KeepRatio in the Image Properties dialog. The problem was found on tests to Bug 143321. Reproducing the problem: - open attached document "Test_Bildgröße_V2_keep_ratio_off.odt" - select the image and open the properties dialog -> KeepRatio is unchecked - check KeepRatio + exit by OK - Result: the Undo arrow is shown blue, Redo arrow is grayed and a red point is added to the Save icon -> OK - click on the Undo arrow - Result: the Undo arrow is grayed, Redo arrow is shown blue and a the red point on the Save icon disappeared -> looks ОК - but: select the image and open the properties dialog again-> KeepRatio is still checked -> not OK! - uncheck KeepRatio -> Height value is set to Width value (10,00 cm) - problem as described in Bug 143321 - reset Height value manually to 7.5 cm + exit by OK - Result: Undo arrow, Redo arrow and Save icon are not changed (gray, blue, without red point) -> ОК (???) - click on the Redo arrow - Result: Undo arrow is shown blue, Redo arrow is grayed and a red point is added to the Save icon -> looks OK - but: select the image and open the properties dialog -> looks like before, KeepRatio is unchecked -> not OK
Confirm Version: 7.3.0.0.alpha0+ (x64) / LibreOffice Community Build ID: 4e0a7df2dfa10bc52d5dbda34f43e0bc0df22ae7 CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: Skia/Raster; VCL: win Locale: nl-NL (nl_NL); UI: en-US Calc: CL
Also in 6.2 and in Versie: 4.4.7.2 Build ID: f3153a8b245191196a4b6b9abd1d0da16eead600 Locale: nl_NL also in Versie: 4.1.0.4 Build ID: 89ea49ddacd9aa532507cbf852f2bb22b1ace28 and in Versie 4.0.0.3 (Bouw-id: 7545bee9c2a0782548772a21bc84a9dcc583b89) no undo in 3.5.7.2 Adding bibisectrequest for the implementation of undo. Suspecting an implementationError
After the last change (buzea.bogdan@libreoffice.org, 2023-05-12 15:57:40 UTC) I have tested with current and coming soon versions of LibreOffice if the bug still exists: Version: 7.5.3.2 (X86_64) / LibreOffice Community Build ID: 9f56dff12ba03b9acd7730a5a481eea045e468f3 CPU threads: 4; OS: Windows 10.0 Build 19045; UI render: default; VCL: win Locale: de-DE (de_DE); UI: de-DE Calc: CL threaded and Version: 7.6.0.0.alpha1+ (X86_64) / LibreOffice Community Build ID: 6f1534940ac12ff8e46f4782e18cfb6cf585da39 CPU threads: 4; OS: Windows 10.0 Build 19045; UI render: Skia/Raster; VCL: win Locale: de-DE (de_DE); UI: de-DE Calc: CL threaded I have tested as decribed in the bug description (open "Test_Bildgröße_V2_keep_ratio_off.odt" and check "KeepRatio" in the picture properties dialog). Result with both LO versions: The behavior has changed completely: - the Undo arrow remains unchanged (gray) - the Save icon remains unchanged (no red point is added) - When I click on the Save icon (without the red point!) the changed document is saved correctly: New entry "KeepRatio" is added to ods subdocument "settings.xml" and at the next open of the document "KeepRatio" appears checked. - But when I try to close the document without save no question appears "Document changed, save?" and the document is closed without to save the change. Because of the changed behavior I am not sure if the bug shall remain opened (status "New") or if it should be closed (what reason?) and a new bug should be reported.
I have bibisected to find the change from "old" behavior as described in the bug description to the "new" behavior as described in Comment 3: e18b47dbd75cec720d7f486a44b8a235625a5856 is the first bad commit commit e18b47dbd75cec720d7f486a44b8a235625a5856 Author: Norbert Thiebaud <nthiebaud@gmail.com> Date: Thu Mar 17 11:22:13 2022 -0700 source 9e8712ed6f9fb5dbd971e352a5709bd45fadc74f source 9e8712ed6f9fb5dbd971e352a5709bd45fadc74f instdir/program/cuilo.dll | Bin 4620800 -> 4621312 bytes instdir/program/setup.ini | 2 +- instdir/program/swlo.dll | Bin 17019392 -> 17019904 bytes instdir/program/version.ini | 2 +- 4 files changed, 2 insertions(+), 2 deletions(-) The "old" behavior was in the bisect tests exactly as described in the bug description except the detail "Height value is set to Width value (10,00 cm) - problem as described in Bug 143321". This effect propably didn't occur because Bug 143633 was already fixed in LO 7.2.1 on 2021-08-25 15:10:09 UTC. IMHO it makes no sense to bibisect for the start of the "old" behavior because it is obsolete.
*** Bug 154162 has been marked as a duplicate of this bug. ***
Let's ask Miklos what he thinks
The UI is a bit messy here, I fear. On one hand, "keep ratio" can be a special value for width or height in case the other axis has a fixed value. On the other hand, this "keep ratio" can be a view option, so if you enabled it for one image, there is some effort to enable it for an other image as well, if you adjust the width or height. You see how "keep ratio" can mean two different things. :-) One is a UI-level option, the other is something that is stored in the document model for an image. The linked commit mentions the "80% shows up as 237%" problem, we certainly don't want to return to the old state. But in case somebody wants to improve the UI here so it's less confusing, that would make sense.
(In reply to Miklos Vajna from comment #7) > The UI is a bit messy here, I fear. On one hand, "keep ratio" can be a > special value for width or height in case the other axis has a fixed value. > > On the other hand, this "keep ratio" can be a view option, so if you enabled > it for one image, there is some effort to enable it for an other image as > well, if you adjust the width or height. > > You see how "keep ratio" can mean two different things. :-) One is a > UI-level option, the other is something that is stored in the document model > for an image. > > The linked commit mentions the "80% shows up as 237%" problem, we certainly > don't want to return to the old state. But in case somebody wants to improve > the UI here so it's less confusing, that would make sense. UX team: could that "somebody" be you?!
When in Comment_7 is written "The UI is a bit messy here ..." I think ths is true. But I am not sure if the information in Comment_7 is completely true. I have seen the same (or nearly the same) properties dialog is used for different object types (images, frames, ???) but especially the "Keep ratio" setting is handled different. As far I have seen until now and just tested newly the setting "Keep ratio" from the Image properties dialog is used for all images (!) of a document, means one sets this value once on or off and this setting is used for all images. Furthermore this setting is saved in the odt subdocument "Settings.xml" as config item, e.g. <config:config-item config:name="KeepRatio" config:type="boolean">true</config:config-item>. So the value is kept when the document is closed and opened again. On the other hand the properties dialog e.g. for frames looking nearly - or completely (?) - like the properties dialog for images also contains the setting "Keep ratio" but when it is set on it is valid only for this one call of the dialog. It is lost at the next dialog call for this frame or other frames and it is also not used for images. Vice versa the "Keep ratio" value set for images is not used for frames.
SwViewOption::IsKeepRatio() is what I had in mind for the view-level option (this is similar to e.g. "show non-printable characters"). And then we also have SwFormatFrameSize::QueryValue() for the document model, where a magic 255% percent for the width or height means "just keep the ratio according to the other axis". Given that we have a single "keep ratio" on the UI, this is confusing, I would say.
The UI-only feature could be visualized differently to ordinary controls. What I have in mind is a icon-only button showing a lock symbol (different icons when toggled on/off) and "connector lines" to the respective controls. Width [ ] -| [ ] Relative... [x] Height [ ] -| Since the "connector lines might be not drawable easily we could just indicate this with an image or on-the-fly created angle. Easy hackable, if accepted. See also bug 80281 - FORMATTING: "Keep aspect ratio" checkbox of images or photos not always honoured concludes to switch the option on by default.
The "Keep ratio" checkbox is found at cui/uiconfig/ui/swpossizepage.ui as well as cui/uiconfig/ui/possizetabpage.ui (Position & Size dialog) cui/uiconfig/ui/linetabpage.ui (Line dialog under very special circumstances) sw/uiconfig/swriter/ui/frmtypepage.ui (frame properties, the lock might be not so appealing here) and in the sidebar svx/uiconfig/ui/sidebarpossize.ui (probably not necessary to change)
Created attachment 191062 [details] Screenshot with https://gerrit.libreoffice.org/c/core/+/159835/5 in place (for discussion in Gerrit)
Created attachment 191115 [details] Alternative solution Things are worse with gen. How about a checkbox with a nice image next to it? (For some reason the image is not shown yet, but I wanted to share the idea.)
(In reply to Heiko Tietze from comment #14) > Things are worse with gen. Both gen and Windows look OK for me now (s. comment in Gerrit, I didn't have the Colibre icon theme included in my build previously). > How about a checkbox with a nice image next to > it? (For some reason the image is not shown yet, but I wanted to share the > idea.) Does that mean to basically combine what the 2 dialogs in attachment 191115 [details] show, i.e. show the icon in addition to (and next to) the checkbox - but the icon now being a plain image instead of a button itself and no longer being keyboard-focusable. Both variants (button with the image, as the current Gerrit change has it, and the checkbox are fine for me and an improvement over the current situation.
(In reply to Michael Weghorn from comment #15) > Does that mean to basically combine what the 2 dialogs in attachment 191115 [details] > [details] show, i.e. show the icon in addition to (and next to) the checkbox > - but the icon now being a plain image instead of a button itself and no > longer being keyboard-focusable. The image in the background is what I have in mind. The checkbox is accessible but without text and the image instead (custom content).
(In reply to Heiko Tietze from comment #16) > (In reply to Michael Weghorn from comment #15) > > Does that mean to basically combine what the 2 dialogs in attachment 191115 [details] > > [details] show, i.e. show the icon in addition to (and next to) the checkbox > > - but the icon now being a plain image instead of a button itself and no > > longer being keyboard-focusable. > The image in the background is what I have in mind. The checkbox is > accessible but without text and the image instead (custom content). Sounds good to me. I suppose that as long as the checkbox has a proper a11y name it should be announced by screen readers.
Question: My original problem was/is that a change of setting "Keep ratio" for images is not correctly handled at Undo/Redo. Is the change of UI necessary to solve this problem resp. is it easier to solve the problem when the UI is changed?
(In reply to Stefan_Lange_KA@T-Online.de from comment #18) > Question: My original problem was/is that a change of setting "Keep ratio" > for images is not correctly handled at Undo/Redo. > Is the change of UI necessary to solve this problem resp. is it easier to > solve the problem when the UI is changed? I don't think so. It appears to me that the bug here got hijacked by mistake for a different purpose.. IMHO The "Improve UI in Image Properties Dialog for the locked state" surely liked, though. I'm unable to find the proper bug. Bug 144195 comment 8 is related and I might asked for lock button it up in context of table column sizes.
(In reply to Telesto from comment #19) > (In reply to Stefan_Lange_KA@T-Online.de from comment #18) > > Question: My original problem was/is that a change of setting "Keep ratio" > > for images is not correctly handled at Undo/Redo. > > Is the change of UI necessary to solve this problem resp. is it easier to > > solve the problem when the UI is changed? > > I don't think so. It appears to me that the bug here got hijacked by mistake > for a different purpose.. IMHO > > The "Improve UI in Image Properties Dialog for the locked state" surely > liked, though. I'm unable to find the proper bug. Bug 144195 comment 8 is > related and I might asked for lock button it up in context of table column > sizes. The title (summary) of this bug originally was "Undo/redo for checking/unchecking KeepRatio in Image Properties Dialog not properly handled". It was changed on 2023-11-22 08:09:18 UTC.
(In reply to Telesto from comment #19) > It appears to me that the bug here got hijacked... Harsh words. The ratio on/off toggle never was and will not be part of the image/object attribute. The "hi-jacking" aims to make this more clear - but yes, it will not change to be UI only. No undo/redo.
(In reply to Heiko Tietze from comment #21) > (In reply to Telesto from comment #19) > > It appears to me that the bug here got hijacked... > Harsh words. The ratio on/off toggle never was and will not be part of the > image/object attribute. The "hi-jacking" aims to make this more clear - but > yes, it will not change to be UI only. No undo/redo. It is correct: The KeepRatio setting for images isn't a part of the image object and - correctly - it isn't saved in odf subdocument "Content.xml". But it is a setting - currently - valid for the whole document and saved as an entry in the odf subdocument "Settings.xml". IMHO therefore the change of this setting should be correctly handled regarding as well Undo and Redo as the internal switch "document changed" + reminder question "Document changed, close?".
(In reply to Heiko Tietze from comment #21) > (In reply to Telesto from comment #19) > > It appears to me that the bug here got hijacked... > Harsh words. FWIW: Not intended to be harsh (or personal). Only to be pointy/direct. It has familiarity with https://en.scratch-wiki.info/wiki/Thread_Hijacking if you ask me. Changing te bug title and scope of a bug without prior notice and/or clarification It can be pretty confusing and/or utterly astonishing for a bug reporter if the bug suddenly being about something different or focussing on a very narrow element of the original report
(In reply to Telesto from comment #23) > It can be pretty confusing and/or utterly astonishing for a bug reporter if > the bug suddenly being about something different or focussing on a very > narrow element of the original report Created bug 158531 and vote for WF here. (In reply to Stefan_Lange_KA@T-Online.de from comment #22) > ...saved as an entry in the odf subdocument "Settings.xml". We shouldn't save this option then.
I think a fresh bug report is needed. The problems have mutated/evolved over time so the initial bug report doesn't match comment 0 (In reply to Stefan_Lange_KA@T-Online.de from comment #9) > As far I have seen until now and just tested newly the setting "Keep ratio" > from the Image properties dialog is used for all images (!) of a document, > means one sets this value once on or off and this setting is used for all > images. True, and undesired in my opinion. You enable something like this for each image individually. It surely shouldn't behave as a global setting. You open a image specific dialog and 'keep ratio' is applied for all images in document.. Note: The same behaviour is also observed in draw If Keep Ratio should be toggled on or off by default another question and be saved for each image independently (if you ask me) and frame for that matter > Furthermore this setting is saved in the odt subdocument > "Settings.xml" as config item, e.g. <config:config-item > config:name="KeepRatio" config:type="boolean">true</config:config-item>. > So the value is kept when the document is closed and opened again. Not OK, should be a image specific setting. Which is my personal preference. Or nothing should be saved at all > On the other hand the properties dialog e.g. for frames looking nearly - or > completely (?) - like the properties dialog for images also contains the > setting "Keep ratio" but when it is set on it is valid only for this one > call of the dialog. It is lost at the next dialog call for this frame or > other frames and it is also not used for images. > Vice versa the "Keep ratio" value set for images is not used for frames. Not storing the keep ratio checkbox is at least coherent. Version: 24.2.0.0.alpha0+ (X86_64) / LibreOffice Community Build ID: 5682e1d4145c26fc8021879df0543d5aeacf9c83 CPU threads: 8; OS: macOS 13.4.1; UI render: Skia/Raster; VCL: osx Locale: nl-NL (nl_NL.UTF-8); UI: en-US Calc: threaded
(In reply to Telesto from comment #25) > I think a fresh bug report is needed. The problems have mutated/evolved over > time so the initial bug report doesn't match comment 0 > > (In reply to Stefan_Lange_KA@T-Online.de from comment #9) > > As far I have seen until now and just tested newly the setting "Keep ratio" > > from the Image properties dialog is used for all images (!) of a document, > > means one sets this value once on or off and this setting is used for all > > images. > > True, and undesired in my opinion. You enable something like this for each > image individually. It surely shouldn't behave as a global setting. You open > a image specific dialog and 'keep ratio' is applied for all images in > document.. > > Note: The same behaviour is also observed in draw > > If Keep Ratio should be toggled on or off by default another question and be > saved for each image independently (if you ask me) and frame for that matter > > > Furthermore this setting is saved in the odt subdocument > > "Settings.xml" as config item, e.g. <config:config-item > > config:name="KeepRatio" config:type="boolean">true</config:config-item>. > > So the value is kept when the document is closed and opened again. > > Not OK, should be a image specific setting. Which is my personal preference. > Or nothing should be saved at all > > > On the other hand the properties dialog e.g. for frames looking nearly - or > > completely (?) - like the properties dialog for images also contains the > > setting "Keep ratio" but when it is set on it is valid only for this one > > call of the dialog. It is lost at the next dialog call for this frame or > > other frames and it is also not used for images. > > Vice versa the "Keep ratio" value set for images is not used for frames. > > Not storing the keep ratio checkbox is at least coherent. > > Version: 24.2.0.0.alpha0+ (X86_64) / LibreOffice Community > Build ID: 5682e1d4145c26fc8021879df0543d5aeacf9c83 > CPU threads: 8; OS: macOS 13.4.1; UI render: Skia/Raster; VCL: osx > Locale: nl-NL (nl_NL.UTF-8); UI: en-US > Calc: threaded I also think this bug is worn-out. But because there are several problems I am not sure if a new bug should be reported or better a proposal for enhancement: - In the ODF Standard V 1.3 there is no image property "Keep aspect ratio". Even if in LibreOffice an individual KeepRatio value for image objects is introduced this setting cannot be saved as image property in a odf document. - Propably it would be better when the "KeepRatio" setting is used only once as it is done in properties dialog for frames. - The other question is how this setting is set initially or as default. IMHO this can be done by the "KeepRatio" value from Settings.xml, and this value could be set by a new Options value instead to take it from a Picture properties dialog.
@Regina Any opinion on comment 26? (In reply to Stefan_Lange_KA@T-Online.de from comment #26) > I also think this bug is worn-out. But because there are several problems I > am not sure if a new bug should be reported or better a proposal for > enhancement: > - In the ODF Standard V 1.3 there is no image property "Keep aspect ratio". > Even if in LibreOffice an individual KeepRatio value for image objects is > introduced this setting cannot be saved as image property in a odf document. > - Propably it would be better when the "KeepRatio" setting is used only once > as it is done in properties dialog for frames. > - The other question is how this setting is set initially or as default. > IMHO this can be done by the "KeepRatio" value from Settings.xml, and this > value could be set by a new Options value instead to take it from a Picture > properties dialog.
Just chiming in to say that this behavior is still present in Version: 24.2.1.2 (X86_64) / LibreOffice Community Build ID: db4def46b0453cc22e2d0305797cf981b68ef5ac CPU threads: 8; OS: Windows 10.0 Build 19045; UI render: default; VCL: win Locale: en-US (en_US); UI: en-US Calc: threaded In this version, however Version: 24.8.0.0.alpha0+ (X86_64) / LibreOffice Community Build ID: a2265e8faa099d9652efd12392c2877c2df1d1eb CPU threads: 8; OS: Windows 10.0 Build 19045; UI render: default; VCL: win Locale: en-US (en_US); UI: en-US Calc: threaded the "Keep Ratio" option is no longer present.
I see the "Keep Ratio" setting no longer as a property that needs to be saved in the document [see also my comment #26]. Because the UI of Image properties dialog is changed in 24.8 (checkbox "Keep ratio" replaced by a lock icon) I propose to put no effort into solving this bug but to close it.
(In reply to Stefan_Lange_KA@T-Online.de from comment #29) > I see the "Keep Ratio" setting no longer as a property that needs to be > saved in the document [see also my comment #26]. Because the UI of Image > properties dialog is changed in 24.8 (checkbox "Keep ratio" replaced by a > lock icon) I propose to put no effort into solving this bug but to close it. Happy you appreciate the modification.
(In reply to Stefan_Lange_KA@T-Online.de from comment #29) > I see the "Keep Ratio" setting no longer as a property that needs to be > saved in the document [see also my comment #26]. Because the UI of Image > properties dialog is changed in 24.8 (checkbox "Keep ratio" replaced by a > lock icon) I propose to put no effort into solving this bug but to close it. Are you sure: 1. open attached document "Test_Bildgröße_V2_keep_ratio_off.odt" 2. select the image and open the properties dialog -> KeepRatio is unchecked 3. check KeepRatio + exit by OK 4. Result: the Undo arrow (gray; disabled)/ Redo arrow (gray: disabled) No red point is added to the Save icon 5. Save 6. select the image and open the properties dialog again-> KeepRatio is still checked -> So apparently saved (as far I can tell)
Back to NEW per clarification.
(In reply to Telesto from comment #31) > (In reply to Stefan_Lange_KA@T-Online.de from comment #29) > > I see the "Keep Ratio" setting no longer as a property that needs to be > > saved in the document [see also my comment #26]. Because the UI of Image > > properties dialog is changed in 24.8 (checkbox "Keep ratio" replaced by a > > lock icon) I propose to put no effort into solving this bug but to close it. > > Are you sure: > > 1. open attached document "Test_Bildgröße_V2_keep_ratio_off.odt" > 2. select the image and open the properties dialog -> KeepRatio is unchecked > 3. check KeepRatio + exit by OK > 4. Result: the Undo arrow (gray; disabled)/ Redo arrow (gray: disabled) No > red point is added to the Save icon > 5. Save > 6. select the image and open the properties dialog again-> KeepRatio is > still checked -> So apparently saved (as far I can tell) It is true: This behavior can be reproduced with LO 24.2 and also with LO 24.8 (changed UI: Lock symbol instead a checkbox)), means the original problem was not solved. But as I have written in Comment 29 I see "Keep Ratio" no longer as a document property necessary to be saved in the document because it doesn't influence the appearance of the document. I see it as a setting that makes easier to change image properties Width and Height in the Image properties dialog. This is like e.g. the autocorrection options (Ignore dubble spaces etc.), also not saved within the document (AFAIK). Therefore I have proposed to close the bug. On the other hand remains the question how this setting should be set initially or as default when the Image properties dialog is called. One way could be to use always a default value - for images e.g. "Height Widht ratio is locked" ("Keep ratio" checked) - means the choosen setting is used only once . An other way could be to keep this setting for further calls and possibly to save it anywhere, e.g. in "Settings.xml" how is realized currently (then valid only for this document) or within the LO options. Nevertheless I see no need to mark the document as changed and to activate "Undo" when the setting is changed. See also my Comment 26! I think this cannot be object of a bug report but e.g a proposal for enhancement.