Bug 98837 - When opening a new writer document, via the api, with read-only mode set to true, writer opens in edit-mode with read-only in the title
Summary: When opening a new writer document, via the api, with read-only mode set to t...
Status: RESOLVED WONTFIX
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
5.2.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: akash96j
URL:
Whiteboard: target:5.2.0 target:5.3.0 target:5.2.2
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-23 10:49 UTC by akash96j
Modified: 2016-08-11 17:04 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:
Regression By:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description akash96j 2016-03-23 10:49:20 UTC

    
Comment 1 akash96j 2016-03-23 10:58:03 UTC
If a new writer document is opened using the api with read-only mode true, writer appends read-only to the title of the document but never sets the document to read-only mode. Also, it does not make sense for an unsaved document to open in read-only mode. So an exception should be thrown if read-only mode is true for a new document.
Comment 2 Buovjaga 2016-04-02 17:11:30 UTC
Please describe the "using the api" bit in more detail.
What operating system are you using?
Did you build 5.2 yourself?

Set to NEEDINFO.
Change back to UNCONFIRMED after you have provided the information.
Comment 3 akash96j 2016-04-02 20:10:39 UTC
Opening a new document with read only property set to true via any API makes writer open a new document with read-only appended in the title, but the document is in edit mode. This bug was found here: 
https://bugs.documentfoundation.org/show_bug.cgi?id=96896#c13

Sample code can be found here(extracted from the bug page above):
https://bug-attachments.documentfoundation.org/attachment.cgi?id=121721

This bug occurs in linux, windows(tested by me). It probably occurs on all other platforms as well. Also I built 5.2 myself.
Comment 4 Commit Notification 2016-04-05 07:03:52 UTC
Akash Jain committed a patch related to this issue.
It has been pushed to "master":

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

tdf#98837 - Fail loading a new document with read only property set to true

It will be available in 5.2.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 5 Stephan Bergmann 2016-06-17 07:18:15 UTC
Bug 100426 comment 7:  "After bug 99827 this is the second known case where the fix for bug 98837 is causing a regression.  Forbidding opening fresh documents as read-only looked like an obvious thing ("who'd want to do that anyway?"), but in light of these regressions it might be better to fix bug 98837 in a different way."
Comment 6 Tor Lillqvist 2016-08-10 11:31:03 UTC
I came across a regression caused by this change: There are some APIs that are for historical reasons defined on a document interface even if they actually affect not the document but LO as a whole. To call such, there was some code that created a dummy empty document using the private:factory/scalc URI and with a ReadOnly parameter as True. This is broken now...

The APIs for which I noticed this were the unpublished ones in css::sheet::opencl::XOpenCLSelection, so yeah, it's "our" (meaning Collabora) own fault for keeping those APIs there even if they now affect LO as a whole... These APIs used to indeed at least in theory affect only a document, but were changed at some point by us to affect LO as a whole.

I wonder if there are more similarly obscure use cases that were broken by this change.
Comment 7 Eike Rathke 2016-08-10 14:02:37 UTC
Maybe the original submitter of this request and author of the patch could explain why opening a new read-only document that in fact was not read-only is a problem, other than UI not being read-only which I don't see as problem because the document could only be saved under a new name anyway, and if it actually was read-only the user couldn't do anything but close an empty document. Furthermore it doesn't make sense to me to present such document to the user. So what's it good for?

Given the problems the fix causes there should be a different approach. Maybe just ignore the read-only request as it was done before but not add read-only to the document title? Hard to say without knowing the original intention, but my impression is this is a "just because" change.
Comment 8 Jan Holesovsky 2016-08-10 14:25:36 UTC
Akash: Sorry about that, but I agree with Eike (and others that are not happy with the change of the behaviour) - we should really revert this :-(

Please try the "normal" read-only case: when you open a read-only document, you'll get the infobar with "this is read only, press the button to edit it" - and when you press the button, you can start editing the document, and yet it still has "(read-only)" in the title.

This is correct - because it marks the _underlying_ storage as read only, ie. that there is no possibility to do Save; but the user can perfectly do a Save As.

The behaviour of the creation of new document is just consistent with this - again marks the underlying doc as read-only, but can edit.  I see no need for changing this.
Comment 9 Tor Lillqvist 2016-08-10 14:47:11 UTC
Reverted now in master, will file cherry-pick requests for 5.2 (and 5.2.1?) in gerrit in a moment.
Comment 10 Commit Notification 2016-08-10 14:49:20 UTC
Tor Lillqvist committed a patch related to this issue.
It has been pushed to "master":

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

Revert "tdf#98837 - Fail loading a new document with read only property set to true"

It will be available in 5.3.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 11 akash96j 2016-08-11 14:36:31 UTC
Hi,

I did not find this bug. This bug was found by someone else during the discussion on a different bug. I just filed it when we found this out.

>Maybe the original submitter of this request and author of the patch could >explain why opening a new read-only document that in fact was not read-only is >a problem, other than UI not being read-only which I don't see as problem >because the document could only be saved under a new name anyway, and if it >actually was read-only the user couldn't do anything but close an empty >document. Furthermore it doesn't make sense to me to present such document to >the user. So what's it good for?

The original problem was that opening a new document via the lo basic api with ReadOnly set to true, opened the document in edit-mode but the UI was in the read-only mode. The doc never opened read-only, even though it is specified in the media descriptor. This was the report:
"When a new doc is opened using loadComponentFromURL("private:factory/swriter","_blank",0,OpenOptions) and OpenOptions has a value "ReadOnly" = True, the expression "(read-only)" is appended to the window title, but document is not put in read-only mode. It is not possible to toggle edit mode using Ctrl+Shift+M (grayed out under Edit menu)."

At that time, it seemed that opening a new document in read-only mode doesn't make sense anyways.
With that being said, I have no problem in reverting this. I agree that this needs to be solved in a different way because it is causing too many regressions with many not being discovered yet.

Thanks Tor for reverting this.
Comment 12 Commit Notification 2016-08-11 17:04:42 UTC
Tor Lillqvist committed a patch related to this issue.
It has been pushed to "libreoffice-5-2":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2f02999c360a4fc55cd5f59eac2beeea630adf32&h=libreoffice-5-2

Revert "tdf#98837 - Fail loading a new document with read only property ..."

It will be available in 5.2.2.

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.