Bug 131296 - Macro for copying data from protected cells and then selecting a different sheet no longer works.
Summary: Macro for copying data from protected cells and then selecting a different sh...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
7.0.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:7.0.0
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2020-03-11 20:57 UTC by Daniel Baran
Modified: 2020-03-25 09:59 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
Macro enabled ODS workbook with partially protected cells. (63.33 KB, application/vnd.oasis.opendocument.spreadsheet)
2020-03-11 21:04 UTC, Daniel Baran
Details
Password-protected macro created in earlier versions does not work in master toward 7.0 unless unlocked (12.35 KB, application/vnd.oasis.opendocument.spreadsheet)
2020-03-18 20:43 UTC, Mike Kaganski
Details
Password-protected macro showing message with an Integer literal (11.09 KB, application/vnd.oasis.opendocument.spreadsheet)
2020-03-19 07:28 UTC, Mike Kaganski
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Baran 2020-03-11 20:57:21 UTC
Description:
The macro copies specific data from protected cells in one sheet, and then selects
a second sheet (changes focus) for pasting the data. This has worked well in multiple earlier releases. In Alpha 7.0.0.0, the correct cells are not copied, and the second sheet is not selected.
It seems the wrong cells are copied and then an attempt is made to past them into the same sheet (not the one specified in the macro). This results in a "Protected cells can not be modified." message.

 

Steps to Reproduce:
1.Open the attached file.
2.Click the "Insert Time" macro button.
3.Click the "Insert Time" button again.
4.Click the "End of Day" macro button.
Focus should jump to Sheet2 and insert zero time values (since less than a minute accrued).


Actual Results:
Focus does not change to Sheet2.
Wrong cells were copied and then an attempt was made to paste them into Sheet1 (wrong sheet)


Expected Results:
Focus should jump to Sheet2 and insert zero time values (since less than a minute accrued).



Reproducible: Always


User Profile Reset: No



Additional Info:
Version: 7.0.0.0.alpha0+ (x64)
Build ID: c63148ba139bd6b9ae7a0f9e24e51f29e5370963
CPU threads: 4; OS: Windows 10.0 Build 18363; UI render: Skia/Raster; VCL: win; 
Locale: en-US (en_US); UI-Language: en-US
Calc: threaded
Comment 1 Daniel Baran 2020-03-11 21:04:36 UTC
Created attachment 158626 [details]
Macro enabled ODS workbook with partially protected cells.

This file can be used for testing this or other bugs as desired.
All embedded macros were functioning properly through LO 6.2.8
Comment 2 Daniel Baran 2020-03-12 01:02:54 UTC
Some additional testing shows that three different macros are not working.
Importantly (I suspect), each of the three involve a jump to another sheet.
The other six macros (that do not include a sheet jump), seem to be working normally.
Comment 3 Xisco Faulí 2020-03-12 09:05:41 UTC
Regression introduced by:

https://cgit.freedesktop.org/libreoffice/core/commit/?id=0b4f8bf571baf2ccd5a8aafdc4deb41867420be3

author	U-DESKTOP-8OSNV7R\DrRobotto <andreas.heinisch@yahoo.de>	2019-12-24 12:22:34 +0100
committer	Mike Kaganski <mike.kaganski@collabora.com>	2020-01-05 21:42:27 +0100
commit 0b4f8bf571baf2ccd5a8aafdc4deb41867420be3 (patch)
tree 5f5af8179e6180f118000df70f49e9783bf66c98
parent 5fbc89478eb91d9b97f0c3d65d9946a5cec31ea3 (diff)
tdf#129596 Distinguish between integer and long while loading immediate values

Bisected with: bibisect-linux64-6.5

Adding Cc: to Andreas Heinisch
Comment 4 Mike Kaganski 2020-03-12 09:23:39 UTC
The problem seems to be related to the password-protected library in the ODS (so a pre-compiled blob is used to execute the macro). As it was created in an older version, the blob is likely not having the data expected by newer code.

The password is required to check what is the original code that fails now (setting NEEDINFO); I suspect that unlocking the library then saving the file would "fix" the problem (which is not a fix for the bug, which likely needs something to enable interoperability both backwards (if possible) and forward; see https://git.libreoffice.org/core/commit/4abb191916916c7003deedcfdcf46287faccaf01 for some inspiration).
Comment 5 Daniel Baran 2020-03-12 17:34:44 UTC
I can confirm Mike Kaganski's suspicion.
When I unprotected the library, the three malfuctioning macros all worked normally.
Happy to provide more info or testing as needed.
Comment 6 Xisco Faulí 2020-03-12 17:36:27 UTC
Hello Daniel,
Could you please provide the password ?
Comment 7 Daniel Baran 2020-03-12 20:57:04 UTC
Additional info has been sent to:
xiscofauli@libreoffice.org
Comment 8 QA Administrators 2020-03-13 03:10:04 UTC Comment hidden (obsolete)
Comment 9 Daniel Baran 2020-03-13 21:06:29 UTC
Just checked for this bug in (pre-release 6.4.2.2):

Version: 6.4.2.2 (x64)
Build ID: 4e471d8c02c9c90f512f7f9ead8875b57fcb1ec3
CPU threads: 4; OS: Windows 10.0 Build 18363; UI render: default; VCL: win; 
Locale: en-US (en_US); UI-Language: en-US
Calc: threaded

Macro execution problem seen in 7.0.0.0 is NOT seen in 6.4.2.2.
No other bugs are evident at this time in this test case (timespreader file).
That is, all macros, computations and data display seem to be as expected when in 6.4.2.2.
Comment 10 Daniel Baran 2020-03-18 20:06:58 UTC
File with unlocked library has been sent to:
xiscofauli@libreoffice.org
and
mike.kaganski@collabora.com
Comment 11 Mike Kaganski 2020-03-18 20:43:16 UTC
Created attachment 158788 [details]
Password-protected macro created in earlier versions does not work in master toward 7.0 unless unlocked

This sample spreadsheet has a library with password "1234" (no quotes).
Comment 12 Mike Kaganski 2020-03-19 06:23:09 UTC
(In reply to Mike Kaganski from comment #11)
> Created attachment 158788 [details]

@DrRobotto: please could you take a look at this? See also comment 4.
Comment 13 Mike Kaganski 2020-03-19 06:29:24 UTC
By the way, the same is true in the opposite direction: unprotecting the library in master toward 7.0, saving the spreadsheet, then opening it in 6.4, the macro does not work correctly  until unprotected.
Comment 14 Mike Kaganski 2020-03-19 07:28:45 UTC
Created attachment 158792 [details]
Password-protected macro showing message with an Integer literal

Even simpler test case: clicking the button must show 123, but shows 0 in master
Comment 15 Mike Kaganski 2020-03-19 09:08:12 UTC
@DrRobotto: to keep the binary compatibility with previous versions, taking into account that the SbiRuntime::StepLOADI is 1-argument operation that takes 32-bit value, but casts it to 16-bit value, we could write old value to lower 16 bits, and string identifier in upper 16 bits in SbiExprNode::Gen for Integer case (which is the problem), and handle that in the SbiRuntime::StepLOADI (if upper 16 bits are 0, then use lower 16 bits).

But then how to tell case SbxLONG? Maybe simply stop generating SbiRuntime::StepLOADI at all, revert its handling to old way (for files created in older versions), and only generate SbiOpcode::NUMBER_ for both Integer and Long? then you'd need to implement type detection in SbiRuntime::StepLOADNC to restore the type? SbiStringPool::Add( double n, SbxDataType t ) could safely write type character after the number, and the code reading the number could take it into account (old code will just read up to non-numeric characters, so would be safe). That would also handle bug 129596 comment 6.
Comment 16 Andreas Heinisch 2020-03-22 17:10:27 UTC
https://gerrit.libreoffice.org/c/core/+/90858
Comment 17 Commit Notification 2020-03-24 08:31:27 UTC
Andreas Heinisch committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/16b0bbb671a8080655e27542e576478486810404

tdf#131296: get numeric value with its data type in StepLOADNC

It will be available in 7.0.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 18 Daniel Baran 2020-03-24 21:56:23 UTC
Confirming the fix in:

Version: 7.0.0.0.alpha0+ (x86)
Build ID: 9163755e9f64a0b1dd5f2090e0702c19e31c12c9
CPU threads: 4; OS: Windows 10.0 Build 18363; UI render: Skia/Raster; VCL: win; 
Locale: en-US (en_US); UI-Language: en-US
Calc: threaded

PS:
Please see report - Bug 131455 (yet unconfirmed).
The same behavior as in 6.4.2.2 is present in this alpha.
Comment 19 Andreas Heinisch 2020-03-24 22:15:11 UTC
Thank you for the feedback! I will investigate your report for tdf#131445
Comment 20 Andreas Heinisch 2020-03-24 22:17:08 UTC
Wrong number sry :( ofc Bug tdf#131455
Comment 21 Xisco Faulí 2020-03-25 09:59:07 UTC
Setting to VERIFIED based on comment 18