Apache OpenOffice (AOO) Bugzilla – Issue 122676
undo-redo works correctly only after double click
Last modified: 2017-05-20 11:41:53 UTC
Created attachment 80993 [details] Attachment showing when undo button clicked 1 time Overview: while executing AOOTest-1429 Undo and Redo functionality works normally as it should be only if Undo and Redo command button clicked 2 times. Steps to Reproduce: 1) Open a new presentation. 2) Insert a drawing object, or a text box, or a fontwork, and keep focus on it 3) change fill type in area section of sidebar ,let say change it to gradient( default fill type is color ) 4) Press undo button. 5) press redo button. Actual Results: After pressing undo button area section showing gradient fill type. Expected Results: After pressing undo button area section should show color fill type. NOTES: Instead of reverting back to color fill type, the fill type remains gradient doesn’t get reverted to its original fill type (color).when clicking 1 more time on undo button the fill type changes to color . Same with redo command.Its also works correctly only if it clicked 2 times to get the expected result. Build Date & Hardware: Build: AOO 4.0 Rev. 1489073 OS: windows 7 Test case Reference :Test Case ID AOOTest-1429 :: Version : 1 Area section - Impress - undo/redo operations
Not sure what is meaning of "area section of sidebar". New presentation Insert picture Change contrast in sidebar to 45% Undo (4% contrast appears, sidebar=properties-graphic) Undo (0% original contrast appears, sidebar=properties-graphic) Undo (blank slide appears, sidebar=properties-layouts) Redo (picture appears, sidebar=properties-layouts) Redo (contrast changes, sidebar=properties-layouts) Redo (contrast changes, sidebar=properties-layouts) Apart from the double click as reported by Prachi, sidebar is stuck during redo in properties-layouts. It is expected it will change to properties-graphic. Rev. 1491860 Debian
ALG: Ih Prachi, I see what you mean. When changing the FillStyle, internally two values will get changed, the FillStyle and the data for it. Each object has a FillStyle and for each FillStyle a set (or defaulted) value. Tus, two undo-actions get created since these actions are executed from the panel individually. There is no easy workaround for this, but of course from user view, this is not correct (even when it is from internal view). I propose to change to enhancement and fix after 4.0 ASAP. Okay?
We will be the laughingstock of the computing world if such a major release will contain this defect.
(In reply to Edwin Sharp from comment #1) > Not sure what is meaning of "area section of sidebar". Missed the attachment, sorry...
ALG: Edwin, from my POV it's only a cosmetic defect. The functionality is not broken, you can (un)do what you intend (even whit more granularity, in principle ;-)). It's not a data loss, not a crash.
ALG: At comment 1: All works as expected. When typing '45' of course you will have a phase with '4'. Maybe a longer wait period wouzld be good for the input fields, but then others will complain. The Redo is not supposed to select the objects (selection is not and was never part of the undos in the graphic parts of the office). The sidebar orients on the selection, thus it's okay from my POV that it stays on what makes sense with the selection (in this case: no selection), it's not 'stuck' in any way.
ALG: Also there is #121924#. That task is for 'merging' the created undos in a useful manner, e.g. in the example from comment 1 it would 'merge' the changes from 'contrast 0->4' and 'contrast 4->45' to one undo in the future. Same is e.g. true for position/rotation/size.
After such a long long wait the public will have high expectations (rightfully so!). Elementary operation like undo-redo must work flawlessly. If needed, release date to be September 1st.
ALG: I see your point, but Undo/Redo works flawlessly, it just has more steps than expected from user's view. Undo/Redo not working flawlessly means crashes or data loss from my POV.
(In reply to Armin Le Grand from comment #9) > Undo/Redo works flawlessly, it just has more > steps than expected from user's view. No one will use a software that is wasting time. The attention span of people nowadays is very short. Uninstall will follow shortly.
remove showstopper request, this is not critical. Easy workaround is possible. I am willing to grant the showstopper flag again when a useable, uncritical fix comes in time. Before you request a showstopper please think about our criteria. We accept only issues that fulfill our showstopper criteria or simple issues where a fix is already in place and which are not critical. And moving the release date based on such simple issue is not an option because than we will never release. And the next release is coming as well.
and keep in mind showstopper rquests have to be checked and this will take time that can't be used to fix real problems.
ALG: Investigated further, I may have found a safe way to do that, need more checks on it... @Edwin: Thanks for your comments and be ensured that I also see that error (as I wrote several times) and want to fix it and do investigate. I think you want to do the right thing, but I have to state that your comments did not increase my motivation (which I have anyways). Please be a little bit more constructive in the future. Thanks!
ALG: The change works with Draw/Impress, checking other apps. Also looking in the processing chain (sfx) if and how this might go wrong..
ALG: Works well in Draw/Calc/Writer and Chart (which has no sidebar currently but is also capable of adding draw objects).
ALG: Using the SfxDispatcher::Execute call which allows a null-terminated list of SfxPoolItems to be handed over instead of doing two calls with just one SfxPoolItem. The difference is that only one SlotID can be handed over, Checked that the adding of SfxPoolItems to an SfxItemSet is directly done in SfxDispatcher::Execute itself, thus the SfxPoolItems always arrive at the application as SfxItemSet. Prerequisite to make this work is that the SlotIDs used lead to the correct execution, this is done in DrawViewShell::FuTemporary and in a single case the slots SID_ATTR_FILL_STYLE, SID_ATTR_FILL_COLOR, SID_ATTR_FILL_GRADIENT, SID_ATTR_FILL_HATCH and SID_ATTR_FILL_BITMAP are handled using SetAttributes at the SdrDrawView. Thus, using a single Execute call with two SfxPoolItems and the slot of the original command and not SID_ATTR_FILL_STYLE works well. This path exists in the other apps, too, so I would say the change is safe. Resetting the ReleaseBlocker flag now with a possible solution which I evaluate as not dangerous. Preparing to add the patch, too.
grant showstopper flag, fix is available
ALG: Played around with various actions and using Undo/Redo massively to evtl. trigger unexpected effects, but found none. Flag is granted, preparing commit.
ALG: Unfortunately changes in AreaProperyPanel conflict with changes on trunk, need checkout and rebuild, that will take a while.
ALG: Okay, build done, checked again, works as expected. Comitting.
ALG: Comitted (r1500087), done.
"alg" committed SVN revision 1500087 into trunk: i122676 Call Execute only once to create only one Undo-ction
Verified on AOO400m3(Build:9702) - Rev. 1502185 2013-07-10 14:15:55 (Mi, 10 Jul 2013) on Win7 OS, Pass