https://bugs.documentfoundation.org/show_bug.cgi?id=104106

Maxim Monastirsky <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #1 from Maxim Monastirsky <[email protected]> ---
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?

-- 
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Libreoffice-bugs mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs

Reply via email to