Bug 56098 - UI: 'Paste Special - Shift Cells' options availability not logical
Summary: UI: 'Paste Special - Shift Cells' options availability not logical
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: Other Windows (All)
: medium normal
Assignee: Winfried Donkers
URL:
Whiteboard: target:4.1.0 target:4.0.3
Keywords:
: 42265 61420 62439 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-10-17 17:45 UTC by Rainer Bielefeld Retired
Modified: 2015-12-21 07:33 UTC (History)
11 users (show)

See Also:
Crash report or crash signature:


Attachments
Sample document (7.70 KB, application/x-vnd.oasis.opendocument.spreadsheet)
2012-10-17 17:45 UTC, Rainer Bielefeld Retired
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Rainer Bielefeld Retired 2012-10-17 17:45:33 UTC
Created attachment 68723 [details]
Sample document

Steps how to reproduce with  "LibreOffice 3.6.3.1” German UI/ German Locale [Build-ID: f8fce0b] on German WIN7 Home Premium (64bit):

0. Download sample document, open it from LibO Start Center File menu
1. Select cells A7:F9 using mouse
2. rightclick cell F9 -> Context menu -> Cut
3. click A4
4. Rightclick A4 -> Context menu -> Space special
   Dialog appears
   Bug: Shift down unavailable, although this would be a required option if you 
        want to get "b" range before "c" range with cut and paste


Second Problem:
10. open sample document from LibO Start Center File menu
11. Select A7:F9 using mouse
12. rightclick cell F9 -> Context menu -> Cut
13. click row heading 4
    > All row 4 selected
14. Rightclick row heading 4 -> Context menu -> Space special
   Dialog appears
   Bug: Shift to right is available, although this can not be done, selecting
        this option + <ok> gives error message 

Third problem
21. Select cells A1:A3 using mouse
22. <control+c> for copy
23. Click Column Header "A"
    > All Column becomes selected
24. <control+v> for paste
    All Column becomes filled with "a"
25. Click into an empty cell
26. Use scroll slider to scroll down to end of page
27. Click last cell in column A
28. Right click -> Context menu -> Paste Special
    Dialog appears
    Bug: 'Shift cells down' now is available althoug that's useless, 
          you can not shift behind las cell

You can also provoke this bug with a completely filled row and trying to paste something with "shift right"

I found this bug during my research for "Bug 39936 - UI: Paste Special not available for paste range"
Comment 1 Rainer Bielefeld Retired 2012-12-21 06:07:11 UTC
Already [Reproducible] with 
* Server Installation of  "LibreOffice 3.6.0.4  German UI/Locale [Build-ID:  932b512] on German WIN7 Home Premium (64bit) 
* [Reproducible] with Server Installation of "LibreOffice 3.3.3  German UI/Locale [OOO330m19 (Build:301) tag libreoffice-3.3.3.1] on German WIN7 Home Premium (64bit) 
* OOo 3.4.0 and OOo 3.1.1, OOo 3.0.2 (only available in menu, not context menu)
  OOo 1.1.5
That never worked, problem is inherited from OOo

NEW for now due to many different versions despite concerns in OOo: In AOOo Bug there were some discussions whether this might be a feature and not a bug, but I can't see any benefit.

@Cor:
do you remember any results concerning discussion in OOo not written in that bug?
Comment 2 Rainer Bielefeld Retired 2012-12-21 06:36:00 UTC
Comment on attachment 68723 [details]
Sample document

Forgot to say: in comment above I only tested fro first problem of original report.
All Problems still [Reproducible] with server  installation of  "LOdev  4.0.0.0.beta1+   -  ENGLISH UI / German Locale  [Build ID: 6d4a55bf38a1c470c49f904dbbddf94eb2f6154)]"  {tinderbox: Win-x86@6, pull time 2012-12-17 08:36:40} on German WIN7 Home Premium (64bit) with own separate User Profile

With this version I experienced something strange concerning first problem in original report. I did the steps, but added 
3.1:  'Rightclick A4 -> Context menu -> Insert...'
     > cells below will be shifted down
3.2: 'Undo Icon'
continue

Now the 'shift down' option is available
Comment 3 Rainer Bielefeld Retired 2012-12-21 06:42:37 UTC
*** Bug 42265 has been marked as a duplicate of this bug. ***
Comment 4 Rainer Bielefeld Retired 2012-12-21 07:05:11 UTC
Only for the sake of completeness, also Second Problem and Third Problem are in all versions back to OOo 1.1.5
Comment 5 Winfried Donkers 2012-12-21 07:23:14 UTC
@Markus:
In January you commented in bug 42265 that you would try to have a look at it. (Probably you didn't find the time to do so, no problem) With Xmas and bad weather looming there _might_ be a possibility for me to have a look at the problem. To avoid double work: did you find anything yet and/or can you give me code pointers?
Comment 6 Winfried Donkers 2012-12-22 16:14:54 UTC
It looks like I have a fix for problem 1 and 2, I want to test it a bit further before I submit as a partial patch.

Problem 3 is not related to 1 and 2 in the code. I should be able to fix this.

I have encountered a 4th problem:
0. use sample document
1. select C4:D6
2. cut
3. select D7
4. Paste Special: shift right is not available - it should be available
5. cancel
6. select E6
7. Paste Special: shift down is not available - it should be available
8. cancel
9. select E7
10. Paste Special: shift down and shift right are available
11. select shift right and paste
12. Undo twice (getting back to situation in step 9.)
13. select D7
14. Paste Special: shift down and shift right are available (!)

I will look into this, but I would like a confirmation that this 4th problem is a problem and occurs not only with me.

(tested on version 4.1.0.0alpha+, openSUSE 12.2)
Comment 7 Winfried Donkers 2012-12-22 18:10:48 UTC
(In reply to comment #0)
> Third problem

This is IMHO not possible to fix efficiently.
If you have a column filled to (almost) MAXROW and you want to paste plus shift down more rows than there are free (the row where you want to paste doesn't matter), you get a neat error message when you try to paste.
To reflect this already in the paste special dialog requiers that the entire sheet must be checked to calculate the available space. I think this will be a) time consuming (not nice) and b) difficult to achieve in the code where the radiobuttons for shift down/right are enabled/disabled (at least for me).

I will submit a patch for problem 1 and 2.
I will see if I can find the cause for problem 4 (and try to fix it).
Comment 8 Rainer Bielefeld Retired 2012-12-22 19:02:33 UTC
(In reply to comment #6)
My results  with "LibreOffice 3.6.4.3" German UI/ German Locale [Build-ID: 2ef5aff] {pull date 2012-11-28} on German WIN7 Home Premium (64bit) :
Your step 4: not reproducible, shift right is available, shift down is not
Your step 7: not reproducible, shift right is NOT available, shift down IS
Your step 10: confirmed
Your step 14: confirmed, that might be something similar as my Comment 2
              Step 3.2

Strange that I see steps 4 and 7 just the opposite of your observations.
For all steps where you said "select" I tried "simple click" (only cell cursor at cell) and really "select", cell is highlighted. Does not matter for me.
Comment 9 Winfried Donkers 2012-12-22 19:40:59 UTC
(In reply to comment #8)
> (In reply to comment #6)
> My results  with "LibreOffice 3.6.4.3" German UI/ German Locale [Build-ID:
> 2ef5aff] {pull date 2012-11-28} on German WIN7 Home Premium (64bit) :
> Your step 4: not reproducible, shift right is available, shift down is not
> Your step 7: not reproducible, shift right is NOT available, shift down IS
> Your step 10: confirmed
> Your step 14: confirmed, that might be something similar as my Comment 2
>               Step 3.2
> 
> Strange that I see steps 4 and 7 just the opposite of your observations.
> For all steps where you said "select" I tried "simple click" (only cell
> cursor at cell) and really "select", cell is highlighted. Does not matter
> for me.

I made exactly the same error as is in the code with respect to problem 1 & 2...
Your observations are correct.
It seems that one or more flags get cleared because of the undo-action(s), causing the code where the buttons are disabled to be skipped. If that indeed is the case (I'm still investigating), I will create a separate bug.

Re. problem 3: I now have a fix that solves problem 1, 2 and 3. Problem 3 is then interpreted as 'the selected range' will not fit on the sheet, so shift ... should be disabled.
Still testing and fine-tuning at the moment.
Comment 10 Rainer Bielefeld Retired 2012-12-23 08:30:33 UTC
> Third problem: This is IMHO not possible to fix efficiently.
No problem at all, the important issue is that "normal" cut and paste with cell shift operations become possible.

Am I right, if we accept to keep problem 3 there is no need at all to have one of the shifts inactive? There is no problem to shift some empty cells to the right ...
Comment 11 Winfried Donkers 2012-12-23 10:41:01 UTC
submitted patch for problems 1, 2, 3 and 4 (step 4 and 7):
https://gerrit.libreoffice.org/1475

Will ask for review for version 3.6/4.0

Will make new bug for problem 4, step 14.
Comment 12 Winfried Donkers 2012-12-23 11:21:04 UTC
(In reply to comment #11)
> Will make new bug for problem 4, step 14.
bug 58678
Comment 13 Winfried Donkers 2013-01-17 15:51:52 UTC
review of submitted patch is delayed because of regression bug 59515 ...
Comment 14 Noel Power 2013-01-18 15:06:18 UTC
I'm pretty sure that http://cgit.freedesktop.org/libreoffice/core/commit/?id=2fe852386c9450014f84910b0a282d684f40b56a

is the culprit
Comment 15 Noel Power 2013-01-18 15:07:12 UTC
(In reply to comment #14)
> I'm pretty sure that.....
sorry updated wrong bug :-( ( still suppose it is related )
Comment 16 Not Assigned 2013-01-24 09:38:26 UTC
Winfried Donkers committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=c2a249f371eb8ac3bf70d92947efeb145566536b

fdo#56098 Paste Special options after cut incorrect



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 17 Winfried Donkers 2013-01-24 10:12:23 UTC
IMHO there if one thing remaining:
With problem 2 and 3 shift options are enabled that lead to a -neat- error message that there is no room for the shift action.

I intend to do the test for this (which takes place in the paste action) before enabling the shift options in the paste special dialog. If this does not incur a performance penalty, I will submit an additional patch for this.
Comment 18 Winfried Donkers 2013-01-24 15:52:17 UTC
I have found the code where the error message originates from :
(/core/sc/source/ui/docshell/docfunc.cxx, ScDocFunc::InsertCells(..)
and /core/sc/source/core/data/document.cxx, ScDocument::InsertCol(..) and InsertRow(..)).

Unfortunately, the process is that when the inserting does not succeed, the action is undone and an error message is presented.
This means that it will be quite an effort to produce the code to just check whether it will fit on the sheet, and I fear that the possible performance penalty will be significant.

Conclusion:
The coding effort plus the performance penalty (to enable/disable the shift buttons in the paste special dialog depending on fitting on the sheet) is IMHO not worth the gain in user friendliness. 
I close this bug as fixed - presuming that Rainer agrees :)
Comment 19 Winfried Donkers 2013-01-27 12:05:01 UTC
Whilst thinking about further refinement, I found a regression problem with my patch (gerrit #1671, comment #16).

There is one situation where shift must be disabled, because of possible data disruption: when the source and destination ranges intersect. With the current code, this is not the case.

The last patch mainly addresses unnecessary disabling of shifting, plus some disabling because the result will not fit on the sheet. This protection because of not fitting is not complete (it will not always be disabled as this will bring with it a large performance penalty in some cases) and may confuse users (why is shift disabled?).
Also, the case of not fitting on the sheet is handled in a user friendly way in the pasting process. The user gets a message telling why the paste is undone.

I am now working on a patch that will make sure that intersecting source/destination ranges disables the shift options and keeps the shift option enabled in all other cases.
With respect to the 4 problem described in this bug:
1: no intersection, shift down & shift right ought to be available;
2: no intersection, shift down & shift right ought to be available, even though there will be an error message when trying to paste@shift right;
3: no intersection, shift down & shift right ought to be available, even though there will be an error message when trying to paste@shift down;
4: no intersection, shift down & shift right ought to be available in all cases.


@Rainer: do you agree from a user point of view?
Comment 20 Winfried Donkers 2013-01-28 15:26:28 UTC
update:
I have got a patch almost ready, which:
-disables the shift options in case of overlap (important, overlap can lead to data distortion/loss). this solves the regression of my previous patch;
-checks if all will fit on the sheet when pasting with shift (the performance penalty seems negligible);
-does this with a single cell selected as destination as well as with a selected range as destination;
-does this for cut as well as copy actions, the shift consequences are the same.

I'm still testing some scenarios and hope to submit the patch soon, maybe tomorrow.
Comment 21 Not Assigned 2013-02-10 05:14:40 UTC
Winfried Donkers committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=5fe7b7561aa78e87f90e8a9436cb674e53ef8f4e

fdo#56098 paste special shift options incorrect/incomplete



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 22 Joel Madero 2013-02-25 04:40:15 UTC
*** Bug 61420 has been marked as a duplicate of this bug. ***
Comment 23 Winfried Donkers 2013-04-03 14:23:54 UTC
*** Bug 62439 has been marked as a duplicate of this bug. ***
Comment 24 Commit Notification 2013-04-05 07:42:07 UTC
Winfried Donkers committed a patch related to this issue.
It has been pushed to "libreoffice-4-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=7f3d167c0ad9859bbd0546ad242e11de65516485&h=libreoffice-4-0

fdo#56098 Paste Special options after cut incorrect


It will be available in LibreOffice 4.0.3.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 25 Commit Notification 2013-04-05 07:42:25 UTC
Winfried Donkers committed a patch related to this issue.
It has been pushed to "libreoffice-4-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=e8ff6eeb29c89f4dd7c77770c088ad4eb8e6fbeb&h=libreoffice-4-0

fdo#56098 paste special shift options incorrect/incomplete


It will be available in LibreOffice 4.0.3.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 26 m.a.riosv 2015-02-03 23:06:27 UTC
*** Bug 89079 has been marked as a duplicate of this bug. ***