Created attachment 128937 [details] test document At least with current master towards LO 5.3 and recent LO 5.2.3: Save the attached embedded-ro.odt as a "physically" read-only file. Open it with LO, in the "This document is open in read-only mode" bar press "Edit Document"; the "Save As" icon remains enabled. Double-click into the "Inner Calc" embedded document to switch it to edit mode. The "Save As" icon erroneously becomes disabled (while "File - Save As..." remains enabled). Single-click into the "Outer Writer" part (to switch the embedded Calc back to non-edit mode), and the "Save As" icon erroneously remains disabled. This worked in LO 5.1 (randomly tested some local LO 5.1.2 build); the disabling of the icon happens in > if ( !m_bReadOnly ) > pToolBox->EnableItem( nId, rEvent.IsEnabled ); in SaveToolbarController::statusChanged (framework/source/uielement/popuptoolbarcontroller.cxx) as added with <https://cgit.freedesktop.org/libreoffice/core/commit/?id=011128aa9493a680c3e9da6d074f125a90ec455c> "SaveToolbarController: back to using XStorable".
Yes, this corner case is *exactly* what the commit message is talking about - "Until a better solution is found, at least make sure that the button is disabled, instead of going into the normal save mode, which doesn't work." i.e. better to have a disabled button, than to have an enabled button that does nothing when clicking it (as it was in 5.1). I will explain: In general this is a "Save" button, not "Save as", and as such is supposed to follow the state of .uno:Save not of .uno:SaveAs. The problem was that .uno:Save is disabled for r/o docs, and if we follow it and disable the toolbar button, the dropdown part won't be accessible as well, although its items (save as, save to remote) might be useful for r/o docs too. But just leaving it always enabled isn't an option - because then clicking the main part of the button while in r/o doc will do nothing (it will try to execute the disabled .uno:Save). So an extra "Save as only" mode was added, where clicking the main part of the button also opens the dropdown, instead of executing .uno:Save. The problem is how to detect the r/o mode. The sfx2 impl. of XStorable::isReadonly returns the UI state, and so was the MediaDescriptor the last time I checked. But .uno:Save doesn't work for "physically" read-only files, regardless of the UI state. Thus again - the button will be enabled, but clicking it will do nothing... I tried to fix it once with 747a0fdda2a7723c2f8a8a022b468bcf29c700e3 ("SaveToolbarController: Better support of readonly docs") but it created other regressions. So I decided to revert it, and instead just disable the button whenever .uno:Save is disabled - as a temporary fix, until something better is implemented. The solution probably should be to make what XStorable::isReadonly returns consistent with the state of .uno:Save. This can be achieved in 2 ways: 1. Make sfx2 impl. of XStorable::isReadonly track the file state rather than the UI state. This will also make it consistent with the IDL doc of XStorable (it wasn't possible to edit "physically" read-only files in OOo, so the way it's implemented wasn't an issue back then). The only problem is that I'm aware of other place in our codebase which depends on the current behavior, so we'll need to expose both somehow (e.g. additional UNO interface, state-only dispatch slot etc.). 2. Make .uno:Save always enabled - but in case of r/o document do actually a "save as". We already do that for the "Save changes to document before closing?" prompt (and also for .uno:Save for new documents, or for file types without an export filter). Any thoughts on what the preferred solution should be?
Another possible solution could be to move the toolbar controller to sfx2, and use sfx2 classes directly...
Maxim Monastirsky committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=afe7f739106124542712ec50dbe2917a8dd14db3 tdf#104106 Enable save whenever edit mode active It will be available in 5.4.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.
Maxim Monastirsky committed a patch related to this issue. It has been pushed to "libreoffice-5-3": http://cgit.freedesktop.org/libreoffice/core/commit/?id=b992da28c595f40a75ddf3237edec10d73e56d91&h=libreoffice-5-3 tdf#104106 Enable save whenever edit mode active It will be available in 5.3.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.