[Libreoffice-commits] online.git: Branch 'distro/collabora/co-4-2' - 3 commits - common/Session.hpp loleaflet/Makefile.am loleaflet/po loleaflet/src wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/Storage.cpp wsd/Storage.hpp wsd/TestStubs.cpp

Mike Kaganski (via logerrit) logerrit at kemper.freedesktop.org
Sun Jul 26 20:13:53 UTC 2020


 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 <mike.kaganski at collabora.com>
AuthorDate: Tue Jul 14 22:19:14 2020 +0300
Commit:     Mike Kaganski <mike.kaganski at collabora.com>
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 <jenkinscollaboraoffice at gmail.com>
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>
    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 <mike.kaganski at collabora.com>
AuthorDate: Tue Jul 14 20:33:13 2020 +0300
Commit:     Mike Kaganski <mike.kaganski at collabora.com>
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 <jenkinscollaboraoffice at gmail.com>
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>
    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 <mike.kaganski at collabora.com>
AuthorDate: Wed Jul 1 11:34:08 2020 +0300
Commit:     Mike Kaganski <mike.kaganski at collabora.com>
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 <jenkinscollaboraoffice at gmail.com>
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>
    (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; }
 


More information about the Libreoffice-commits mailing list