| Summary: | "Save As" icon disabled as soon as switching embedded doc within r/o doc into edit mode | ||
|---|---|---|---|
| Product: | LibreOffice | Reporter: | Stephan Bergmann <sberg.fun> |
| Component: | framework | Assignee: | Maxim Monastirsky <momonasmon> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | momonasmon |
| Priority: | medium | Keywords: | regression |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | target:5.4.0 target:5.3.0.1 | ||
| Crash report or crash signature: | Regression By: | ||
| Attachments: | test document | ||
|
Description
Stephan Bergmann
2016-11-22 11:42:58 UTC
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. |