Bug 108017 - XLSB: sheet protection is reversed
Summary: XLSB: sheet protection is reversed
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
4.3.0.4 release
Hardware: All All
: medium normal
Assignee: Justin L
QA Contact:
URL:
Whiteboard: target:5.5.0 target:5.4.0.1 target:5.3.4
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2017-05-23 08:44 UTC by Justin L
Modified: 2017-05-27 08:49 UTC (History)
1 user (show)

See Also:
Crash report or crash signature:


Attachments
protectedSheet.xlsb: binary format reverses protection settings. (9.19 KB, application/vnd.ms-excel.sheet.binary.macroEnabled.12)
2017-05-23 08:44 UTC, Justin L
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Justin L 2017-05-23 08:44:11 UTC
Created attachment 133466 [details]
protectedSheet.xlsb: binary format reverses protection settings.

This .xlsb file was created with sheet protection enabled. Everything is disallowed, EXCEPT for allowing the selection of unlocked cells.  It works fine in .xlsx format, but in .xlsb the entire sheet is selection-locked EXCEPT for the locked cells. In other words, the protection works backwards for .xlsb compared to .xlsx.

broken in LO4.3 development by author Noel Power 2013-11-21 18:12:50
commit 5728a56b20e01d6c9e86a23f20af64a1b68ad604
fdo#70499 selectn of lock/unlocked cells with worksheet protection reversed
https://cgit.freedesktop.org/libreoffice/core/commit/?id=5728a56b20e01d6c9e86a23f20af64a1b68ad604

Steps to reproduce.
1.) Open protectedSheet.xlsb. Try clicking in a white (unlocked) cell.
2.) Notice that you CAN'T select any of these cells.
3.) click on one of the blue cells (A1:B2) which are locked.
4.) Notice that you CAN select these cells.
5.) Disable tools-Protect Sheet, and then re-enable it again to see the dialog.
6.) Notice that the user can do everything except select unprotected cells.  That is the opposite of what happens in MSO (and the opposite of what happens when reading the .xlsx/.xlsm form of the file).
Comment 1 Xisco Faulí 2017-05-23 09:03:07 UTC
Moving to NEW since the problematic commit has been identified.
Comment 2 Justin L 2017-05-23 19:10:09 UTC
proposed fix at https://gerrit.libreoffice.org/37952
Comment 3 Commit Notification 2017-05-25 18:46:36 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

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

tdf#108017 xlsb import: protection settings were reversed

It will be available in 5.5.0.

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 4 Commit Notification 2017-05-26 04:23:45 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-5-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=47f3d3521c364a9133da26d3202225dc414662fb&h=libreoffice-5-4

tdf#108017 xlsb import: protection settings were reversed

It will be available in 5.4.0.1.

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 5 Commit Notification 2017-05-26 10:41:03 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "libreoffice-5-3":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=4fbb8ad023c95266ce0882cd28bdf6703f5b67bf&h=libreoffice-5-3

tdf#108017 xlsb import: protection settings were reversed

It will be available in 5.3.4.

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 6 Justin L 2017-05-27 08:49:02 UTC
I also looked into ::importChartProtection() since it also appears to be backwards. However that one is confusing because it is hard to know what is an "object" on a chart sheet, and what is content.  Plus, when you turn off sheet protection and then re-enable it, the lock screen returns to the defaults, and not the previous setting, so that makes it even harder to troubleshoot.

Ultimately, at this point LO doesn't support a separate "chartsheets/sheet1.xml" anyway, so it gets converted into normal sheet protection.

Also worth noting is that LO doesn't support chartsheet "contents" locking.  It uses this value to determine whether to lock the sheet instead.

Too confusing and irrelevant to waste any more time on it, so just documenting the fact that I did notice the similar settings in importChartProtection and why I didn't do anything about it.