From 5cbbcbcee6899828f64e8333ee791dbfd3c44774 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 28 Oct 2021 15:55:47 -0400 Subject: [PATCH] pad: Simplify reload after `.etherpad` import The old "switch to pad" logic looked buggy, and it complicates pad initialization. Forcing a refresh after importing an `.etherpad` file isn't much of a UX downgrade. --- CHANGELOG.md | 1 + doc/api/hooks_server-side.md | 6 ++-- src/node/handler/ImportHandler.js | 4 +-- src/node/handler/PadMessageHandler.js | 40 --------------------------- src/static/js/pad.js | 40 ++------------------------- src/static/js/pad_impexp.js | 2 +- 6 files changed, 10 insertions(+), 83 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14114580a..a0f111d4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ ### Notable enhancements +* Simplified pad reload after importing an `.etherpad` file. * For plugin authors: * `clientVars` was added to the context for the `postAceInit` client-side hook. Plugins should use this instead of the `clientVars` global variable. diff --git a/doc/api/hooks_server-side.md b/doc/api/hooks_server-side.md index c46da350c..2f948a790 100644 --- a/doc/api/hooks_server-side.md +++ b/doc/api/hooks_server-side.md @@ -579,9 +579,9 @@ Things in context: This hook allows plugins to grant temporary write access to a pad. It is called for each incoming message from a client. If write access is granted, it applies to the current message and all future messages from the same socket.io -connection until the next `CLIENT_READY` or `SWITCH_TO_PAD` message. Read-only -access is reset **after** each `CLIENT_READY` or `SWITCH_TO_PAD` message, so -granting write access has no effect for those message types. +connection until the next `CLIENT_READY` message. Read-only access is reset +**after** each `CLIENT_READY` message, so granting write access has no effect +for those message types. The handleMessageSecurity function must return a Promise. If the Promise resolves to `true`, write access is granted as described above. Returning diff --git a/src/node/handler/ImportHandler.js b/src/node/handler/ImportHandler.js index acaaf0927..7cc62e113 100644 --- a/src/node/handler/ImportHandler.js +++ b/src/node/handler/ImportHandler.js @@ -235,8 +235,8 @@ const doImport = async (req, res, padId) => { pad = await padManager.getPad(padId); padManager.unloadPad(padId); - // direct Database Access means a pad user should perform a switchToPad - // and not attempt to receive updated pad data + // Direct database access means a pad user should reload the pad and not attempt to receive + // updated pad data. if (directDatabaseAccess) return true; // tell clients to update diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js index 63ef45bae..ead00a3b5 100644 --- a/src/node/handler/PadMessageHandler.js +++ b/src/node/handler/PadMessageHandler.js @@ -283,8 +283,6 @@ exports.handleMessage = async (socket, message) => { } else { messageLogger.warn(`Dropped message, unknown COLLABROOM Data Type ${message.data.type}`); } - } else if (message.type === 'SWITCH_TO_PAD') { - await handleSwitchToPad(socket, message, authorID); } else { messageLogger.warn(`Dropped message, unknown Message Type ${message.type}`); } @@ -806,44 +804,6 @@ const _correctMarkersInPad = (atext, apool) => { return builder.toString(); }; -const handleSwitchToPad = async (socket, message, _authorID) => { - const currentSessionInfo = sessioninfos[socket.id]; - const padId = currentSessionInfo.padId; - - // Check permissions for the new pad. - const newPadIds = await readOnlyManager.getIds(message.padId); - const {session: {user} = {}} = socket.client.request; - const {accessStatus, authorID} = await securityManager.checkAccess( - newPadIds.padId, message.sessionID, message.token, user); - if (accessStatus !== 'grant') { - // Access denied. Send the reason to the user. - socket.json.send({accessStatus}); - return; - } - // The same token and session ID were passed to checkAccess in handleMessage, so this second call - // to checkAccess should return the same author ID. - assert(authorID === _authorID); - assert(authorID === currentSessionInfo.author); - - // Check if the connection dropped during the access check. - if (sessioninfos[socket.id] !== currentSessionInfo) return; - - // clear the session and leave the room - _getRoomSockets(padId).forEach((socket) => { - const sinfo = sessioninfos[socket.id]; - if (sinfo && sinfo.author === currentSessionInfo.author) { - // fix user's counter, works on page refresh or if user closes browser window and then rejoins - sessioninfos[socket.id] = {}; - socket.leave(padId); - } - }); - - // start up the new pad - const newSessionInfo = sessioninfos[socket.id]; - createSessionInfoAuth(newSessionInfo, message); - await handleClientReady(socket, message, authorID); -}; - // Creates/replaces the auth object in the given session info. const createSessionInfoAuth = (sessionInfo, message) => { // Remember this information since we won't diff --git a/src/static/js/pad.js b/src/static/js/pad.js index 35f783db6..20c79af55 100644 --- a/src/static/js/pad.js +++ b/src/static/js/pad.js @@ -48,8 +48,6 @@ const socketio = require('./socketio'); const hooks = require('./pluginfw/hooks'); -let receivedClientVars = false; - // This array represents all GET-parameters which can be used to change a setting. // name: the parameter-name, eg `?noColors=true` => `noColors` // checkVal: the callback is only executed when @@ -181,8 +179,7 @@ const getUrlVars = () => { return vars; }; -const sendClientReady = (isReconnect, messageType) => { - messageType = typeof messageType !== 'undefined' ? messageType : 'CLIENT_READY'; +const sendClientReady = (isReconnect) => { let padId = document.location.pathname.substring(document.location.pathname.lastIndexOf('/') + 1); // unescape neccesary due to Safari and Opera interpretation of spaces padId = decodeURIComponent(padId); @@ -201,7 +198,7 @@ const sendClientReady = (isReconnect, messageType) => { const msg = { component: 'pad', - type: messageType, + type: 'CLIENT_READY', padId, sessionID: Cookies.get('sessionID'), token, @@ -218,6 +215,7 @@ const sendClientReady = (isReconnect, messageType) => { }; const handshake = () => { + let receivedClientVars = false; let padId = document.location.pathname.substring(document.location.pathname.lastIndexOf('/') + 1); // unescape neccesary due to Safari and Opera interpretation of spaces padId = decodeURIComponent(padId); @@ -394,38 +392,6 @@ const pad = { getUserId: () => pad.myUserInfo.userId, getUserName: () => pad.myUserInfo.name, userList: () => paduserlist.users(), - switchToPad: (padId) => { - let newHref = new RegExp(/.*\/p\/[^/]+/).exec(document.location.pathname) || clientVars.padId; - newHref = newHref[0]; - - const options = clientVars.padOptions; - if (typeof options !== 'undefined' && options != null) { - const optionArr = []; - $.each(options, (k, v) => { - const str = `${k}=${v}`; - optionArr.push(str); - }); - const optionStr = optionArr.join('&'); - - newHref = `${newHref}?${optionStr}`; - } - - // destroy old pad from DOM - // See https://github.com/ether/etherpad-lite/pull/3915 - // TODO: Check if Destroying is enough and doesn't leave negative stuff - // See ace.js "editor.destroy" for a reference of how it was done before - $('#editorcontainer').find('iframe')[0].remove(); - - if (window.history && window.history.pushState) { - $('#chattext p').remove(); // clear the chat messages - window.history.pushState('', '', newHref); - receivedClientVars = false; - sendClientReady(false, 'SWITCH_TO_PAD'); - } else { - // fallback - window.location.href = newHref; - } - }, sendClientMessage: (msg) => { pad.collabClient.sendClientMessage(msg); }, diff --git a/src/static/js/pad_impexp.js b/src/static/js/pad_impexp.js index 8689b5b35..c85a791eb 100644 --- a/src/static/js/pad_impexp.js +++ b/src/static/js/pad_impexp.js @@ -67,7 +67,7 @@ const padimpexp = (() => { importErrorMessage(message); } else { $('#import_export').removeClass('popup-show'); - if (directDatabaseAccess) pad.switchToPad(clientVars.padId); + if (directDatabaseAccess) window.location.reload(); } $('#importsubmitinput').removeAttr('disabled').val(html10n.get('pad.impexp.importbutton')); window.setTimeout(() => $('#importfileinput').removeAttr('disabled'), 0);