common/Session.hpp | 2 - loleaflet/Makefile.am | 1 loleaflet/po/templates/loleaflet-ui.pot | 14 +++++++ loleaflet/src/control/Permission.js | 63 +++++++++++++++++++++++++++----- loleaflet/src/core/Socket.js | 4 ++ wsd/ClientSession.cpp | 39 +++++++++++++++++-- wsd/ClientSession.hpp | 10 ++++- wsd/DocumentBroker.cpp | 15 +++++++ wsd/DocumentBroker.hpp | 3 + wsd/Storage.cpp | 4 ++ wsd/Storage.hpp | 2 + wsd/TestStubs.cpp | 2 - 12 files changed, 143 insertions(+), 16 deletions(-)
New commits: commit 2ee2c1697529e4f3e5349ecc387ab72d271d62f7 Author: Mike Kaganski <[email protected]> AuthorDate: Tue Jul 14 22:19:14 2020 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Sun Jul 26 22:13:49 2020 +0200 Allow user to try to lock the document for edit Use mobile-edit-button for that is permitted. Change-Id: I4d4c3f21d574abae033bacc69def96aaf6b51567 Reviewed-on: https://gerrit.libreoffice.org/c/online/+/98786 Tested-by: Jenkins CollaboraOffice <[email protected]> Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/online/+/99322 diff --git a/loleaflet/po/templates/loleaflet-ui.pot b/loleaflet/po/templates/loleaflet-ui.pot index 2b770af3a..24a8c9c37 100644 --- a/loleaflet/po/templates/loleaflet-ui.pot +++ b/loleaflet/po/templates/loleaflet-ui.pot @@ -819,16 +819,20 @@ msgstr "" msgid "Current" msgstr "" -#: src/control/Permission.js:42 +#: src/control/Permission.js:45 msgid "The document could not be locked, and is opened in read-only mode." msgstr "" -#: src/control/Permission.js:44 +#: src/control/Permission.js:47 src/control/Permission.js:65 msgid "" "\n" "Server returned this reason: \"" msgstr "" +#: src/control/Permission.js:63 +msgid "The document could not be locked." +msgstr "" + #: src/control/Ruler.js:368 msgid "Left Margin" msgstr "" diff --git a/loleaflet/src/control/Permission.js b/loleaflet/src/control/Permission.js index 68bc731ea..b5a19439c 100644 --- a/loleaflet/src/control/Permission.js +++ b/loleaflet/src/control/Permission.js @@ -13,24 +13,22 @@ L.Map.include({ var that = this; button.on('click', function () { - button.hide(); - that._enterEditMode('edit'); - that.fire('editorgotfocus'); - // In the iOS/android app, just clicking the mobile-edit-button is - // not reason enough to pop up the on-screen keyboard. - if (!(window.ThisIsTheiOSApp || window.ThisIsTheAndroidApp)) - that.focus(); + that._switchToEditMode(); }); // temporarily, before the user touches the floating action button this._enterReadOnlyMode('readonly'); } + else if (this.options.canTryLock) { + // This is a success response to an attempt to lock using mobile-edit-button + this._switchToEditMode(); + } else { this._enterEditMode(perm); } } else if (perm === 'view' || perm === 'readonly') { - if (window.mode.isMobile() || window.mode.isTablet()) { + if (!this.options.canTryLock && (window.mode.isMobile() || window.mode.isTablet())) { $('#mobile-edit-button').hide(); } @@ -39,13 +37,50 @@ L.Map.include({ }, onLockFailed: function(reason) { - var alertMsg = _('The document could not be locked, and is opened in read-only mode.'); - if (reason) { - alertMsg += _('\nServer returned this reason: "') + reason + '"'; + if (this.options.canTryLock === undefined) { + // This is the initial notification. This status is not permanent. + // Allow to try to lock the file for edit again. + this.options.canTryLock = true; + + var alertMsg = _('The document could not be locked, and is opened in read-only mode.'); + if (reason) { + alertMsg += _('\nServer returned this reason: "') + reason + '"'; + } + vex.dialog.alert({ message: alertMsg }); + + var button = $('#mobile-edit-button'); + // TODO: modify the icon here + button.show(); + button.off('click'); + + var that = this; + button.on('click', function () { + that._socket.sendMessage('attemptlock'); + }); } + else if (this.options.canTryLock) { + // This is a failed response to an attempt to lock using mobile-edit-button + alertMsg = _('The document could not be locked.'); + if (reason) { + alertMsg += _('\nServer returned this reason: "') + reason + '"'; + } + vex.dialog.alert({ message: alertMsg }); + } + // do nothing if this.options.canTryLock is defined and is false + }, - vex.dialog.alert({ message: alertMsg }); - this.options.canTryLock = true; + // from read-only to edit mode + _switchToEditMode: function () { + this.options.canTryLock = false; // don't respond to lockfailed anymore + $('#mobile-edit-button').hide(); + this._enterEditMode('edit'); + if (window.mode.isMobile() || window.mode.isTablet()) { + this.fire('editorgotfocus'); + // In the iOS/android app, just clicking the mobile-edit-button is + // not reason enough to pop up the on-screen keyboard. + if (!(window.ThisIsTheiOSApp || window.ThisIsTheAndroidApp)) + this.focus(); + } }, _enterEditMode: function (perm) { diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp index 9ae340c71..a03bf5927 100644 --- a/wsd/ClientSession.cpp +++ b/wsd/ClientSession.cpp @@ -446,7 +446,8 @@ bool ClientSession::_handleInput(const char *buffer, int length) tokens[0] != "removetextcontext" && tokens[0] != "dialogevent" && tokens[0] != "completefunction" && - tokens[0] != "formfieldevent") + tokens[0] != "formfieldevent" && + tokens[0] != "attemptlock") { LOG_ERR("Session [" << getId() << "] got unknown command [" << tokens[0] << "]."); sendTextFrameAndLogError("error: cmd=" + tokens[0] + " kind=unknown"); @@ -735,6 +736,10 @@ bool ClientSession::_handleInput(const char *buffer, int length) { return forwardToChild(firstLine, docBroker); } + else if (tokens.equals(0, "attemptlock")) + { + return attemptLock(docBroker); + } else { if (tokens.equals(0, "key")) @@ -1006,6 +1011,24 @@ void ClientSession::setLockFailed(const std::string& sReason) sendTextFrame("lockfailed:" + sReason); } +bool ClientSession::attemptLock(const std::shared_ptr<DocumentBroker>& docBroker) +{ + if (!isReadOnly()) + return true; + // We are only allowed to change into edit mode if the read-only mode is because of failed lock + if (!_isLockFailed) + return false; + + std::string failReason; + const bool bResult = docBroker->attemptLock(*this, failReason); + if (bResult) + setReadOnly(false); + else + sendTextFrame("lockfailed:" + failReason); + + return bResult; +} + bool ClientSession::hasQueuedMessages() const { return _senderQueue.size() > 0; diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp index 89f86cf02..810c71156 100644 --- a/wsd/ClientSession.hpp +++ b/wsd/ClientSession.hpp @@ -221,6 +221,9 @@ private: void handleTileInvalidation(const std::string& message, const std::shared_ptr<DocumentBroker>& docBroker); + /// If this session is read-only because of failed lock, try to unlock and make it read-write. + bool attemptLock(const std::shared_ptr<DocumentBroker>& docBroker); + private: std::weak_ptr<DocumentBroker> _docBroker; diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 164a2126e..44edf9879 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -916,6 +916,15 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s return true; } +bool DocumentBroker::attemptLock(const ClientSession& session, std::string& failReason) +{ + const bool bResult = _storage->updateLockState(session.getAuthorization(), session.getCookies(), + *_lockCtx, true); + if (!bResult) + failReason = _lockCtx->_lockFailureReason; + return bResult; +} + bool DocumentBroker::saveToStorage(const std::string& sessionId, bool success, const std::string& result, bool force) { diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp index 17300d0cb..e428924ac 100644 --- a/wsd/DocumentBroker.hpp +++ b/wsd/DocumentBroker.hpp @@ -170,6 +170,9 @@ public: /// Notify that the load has completed virtual void setLoaded(); + /// If not yet locked, try to lock + bool attemptLock(const ClientSession& session, std::string& failReason); + bool isDocumentChangedInStorage() { return _documentChangedInStorage; } /// Save the document to Storage if it needs persisting. commit 1c7045b2978b1d566ddc5f55abae34c72c342b92 Author: Mike Kaganski <[email protected]> AuthorDate: Tue Jul 14 20:33:13 2020 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Sun Jul 26 22:13:42 2020 +0200 Warn user when the document could not be locked Change-Id: I66b584f5e95fd82dc5cb27d10d9629cf19cb61bd Reviewed-on: https://gerrit.libreoffice.org/c/online/+/98782 Tested-by: Jenkins CollaboraOffice <[email protected]> Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/online/+/99321 diff --git a/loleaflet/Makefile.am b/loleaflet/Makefile.am index 90e72ef39..6d0cea8fe 100644 --- a/loleaflet/Makefile.am +++ b/loleaflet/Makefile.am @@ -578,6 +578,7 @@ pot: src/control/Control.NotebookbarCalc.js \ src/control/Control.NotebookbarImpress.js \ src/control/Control.NotebookbarBuilder.js \ + src/control/Permission.js \ src/control/Ruler.js \ src/control/Signing.js \ src/control/Toolbar.js \ diff --git a/loleaflet/po/templates/loleaflet-ui.pot b/loleaflet/po/templates/loleaflet-ui.pot index 4db984722..2b770af3a 100644 --- a/loleaflet/po/templates/loleaflet-ui.pot +++ b/loleaflet/po/templates/loleaflet-ui.pot @@ -819,6 +819,16 @@ msgstr "" msgid "Current" msgstr "" +#: src/control/Permission.js:42 +msgid "The document could not be locked, and is opened in read-only mode." +msgstr "" + +#: src/control/Permission.js:44 +msgid "" +"\n" +"Server returned this reason: \"" +msgstr "" + #: src/control/Ruler.js:368 msgid "Left Margin" msgstr "" diff --git a/loleaflet/src/control/Permission.js b/loleaflet/src/control/Permission.js index e8fe959ca..68bc731ea 100644 --- a/loleaflet/src/control/Permission.js +++ b/loleaflet/src/control/Permission.js @@ -2,7 +2,7 @@ /* * Document permission handler */ -/* global $ */ +/* global $ _ vex */ L.Map.include({ setPermission: function (perm) { if (perm === 'edit') { @@ -38,6 +38,16 @@ L.Map.include({ } }, + onLockFailed: function(reason) { + var alertMsg = _('The document could not be locked, and is opened in read-only mode.'); + if (reason) { + alertMsg += _('\nServer returned this reason: "') + reason + '"'; + } + + vex.dialog.alert({ message: alertMsg }); + this.options.canTryLock = true; + }, + _enterEditMode: function (perm) { if (this._permission == 'readonly' && (window.mode.isMobile() || window.mode.isTablet())) { this.sendInitUNOCommands(); diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js index 80decbee2..f950194cc 100644 --- a/loleaflet/src/core/Socket.js +++ b/loleaflet/src/core/Socket.js @@ -334,6 +334,10 @@ L.Socket = L.Class.extend({ return; } + else if (textMsg.startsWith('lockfailed:')) { + this._map.onLockFailed(textMsg.substring('lockfailed:'.length).trim()); + return; + } else if (textMsg.startsWith('wopi: ')) { // Handle WOPI related messages var wopiInfo = JSON.parse(textMsg.substring(textMsg.indexOf('{'))); commit a51d9c2ff79290aa7f548616a4b75b48440ab088 Author: Mike Kaganski <[email protected]> AuthorDate: Wed Jul 1 11:34:08 2020 +0300 Commit: Mike Kaganski <[email protected]> CommitDate: Sun Jul 26 22:13:34 2020 +0200 Handle failed locking as (temporarily) read-only session E.g., opening a checked-out document in SharePoint Change-Id: Ifd5225d8450d7f2f5ba9661f158551c5c16f9b09 Reviewed-on: https://gerrit.libreoffice.org/c/online/+/97596 Tested-by: Jenkins CollaboraOffice <[email protected]> Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> (cherry picked from commit e9c4c0286af8e139dd2c3d4bddfcf9d3736e6e1e) Reviewed-on: https://gerrit.libreoffice.org/c/online/+/99298 diff --git a/common/Session.hpp b/common/Session.hpp index 02a11fa23..34a566b3c 100644 --- a/common/Session.hpp +++ b/common/Session.hpp @@ -69,7 +69,7 @@ public: const std::string& getName() const { return _name; } bool isDisconnected() const { return _disconnected; } - virtual void setReadOnly() { _isReadOnly = true; } + virtual void setReadOnly(bool bVal = true) { _isReadOnly = bVal; } bool isReadOnly() const { return _isReadOnly; } /// overridden to prepend client ids on messages by the Kit diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp index 904867b18..9ae340c71 100644 --- a/wsd/ClientSession.cpp +++ b/wsd/ClientSession.cpp @@ -991,11 +991,19 @@ bool ClientSession::filterMessage(const std::string& message) const return allowed; } -void ClientSession::setReadOnly() +void ClientSession::setReadOnly(bool bVal) { - Session::setReadOnly(); + Session::setReadOnly(bVal); // Also inform the client - sendTextFrame("perm: readonly"); + const std::string sPerm = bVal ? "readonly" : "edit"; + sendTextFrame("perm: " + sPerm); +} + +void ClientSession::setLockFailed(const std::string& sReason) +{ + _isLockFailed = true; + setReadOnly(); + sendTextFrame("lockfailed:" + sReason); } bool ClientSession::hasQueuedMessages() const diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp index 6103d0550..89f86cf02 100644 --- a/wsd/ClientSession.hpp +++ b/wsd/ClientSession.hpp @@ -38,7 +38,8 @@ public: void construct(); virtual ~ClientSession(); - void setReadOnly() override; + void setReadOnly(bool bVal = true) override; + void setLockFailed(const std::string& sReason); enum SessionState { DETACHED, // initial @@ -232,6 +233,10 @@ private: /// Whether this session is the owner of currently opened document bool _isDocumentOwner; + /// If it is allowed to try to switch from read-only to edit mode, + /// because it's read-only just because of transient lock failure. + bool _isLockFailed = false; + /// The socket to which the converted (saveas) doc is sent. std::shared_ptr<StreamSocket> _saveAsSocket; diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 086ad1699..164a2126e 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -788,9 +788,15 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s session->getAuthorization(), session->getCookies(), *_lockCtx, templateSource); // Only lock the document on storage for editing sessions + // FIXME: why not lock before loadStorageFileToLocal? Would also prevent race conditions if (!session->isReadOnly() && !_storage->updateLockState(session->getAuthorization(), session->getCookies(), *_lockCtx, true)) + { LOG_ERR("Failed to lock!"); + session->setLockFailed(_lockCtx->_lockFailureReason); + // TODO: make this "read-only" a special one with a notification (infobar? balloon tip?) + // and a button to unlock + } #if !MOBILEAPP // Check if we have a prefilter "plugin" for this document format diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp index 3acbccbc4..7f79be098 100644 --- a/wsd/Storage.cpp +++ b/wsd/Storage.cpp @@ -764,6 +764,7 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const Au bool WopiStorage::updateLockState(const Authorization& auth, const std::string& cookies, LockContext& lockCtx, bool lock) { + lockCtx._lockFailureReason.clear(); if (!lockCtx._supportsLocks) return true; @@ -820,7 +821,10 @@ bool WopiStorage::updateLockState(const Authorization& auth, const std::string& { std::string sMoreInfo = response.get("X-WOPI-LockFailureReason", ""); if (!sMoreInfo.empty()) + { + lockCtx._lockFailureReason = sMoreInfo; sMoreInfo = ", failure reason: \"" + sMoreInfo + "\""; + } LOG_WRN("Un-successful " << wopiLog << " with status " << response.getStatus() << sMoreInfo << " and response: " << responseString); } diff --git a/wsd/Storage.hpp b/wsd/Storage.hpp index e8795ab85..e93611222 100644 --- a/wsd/Storage.hpp +++ b/wsd/Storage.hpp @@ -37,6 +37,8 @@ struct LockContext std::string _lockToken; /// Time of last successful lock (re-)acquisition std::chrono::steady_clock::time_point _lastLockTime; + /// Reason for unsuccessful locking request + std::string _lockFailureReason; LockContext() : _supportsLocks(false), _isLocked(false) { } diff --git a/wsd/TestStubs.cpp b/wsd/TestStubs.cpp index ca04416da..4917fd18d 100644 --- a/wsd/TestStubs.cpp +++ b/wsd/TestStubs.cpp @@ -33,7 +33,7 @@ void ClientSession::writeQueuedMessages() {} void ClientSession::dumpState(std::ostream& /*os*/) {} -void ClientSession::setReadOnly() {} +void ClientSession::setReadOnly(bool) {} bool ClientSession::_handleInput(const char* /*buffer*/, int /*length*/) { return false; } _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
