From 0bd41696632f3d15d664253fdf598d6b9746bcff Mon Sep 17 00:00:00 2001 From: Luiza Pagliari Date: Wed, 3 May 2017 12:59:57 -0300 Subject: [PATCH 1/4] [fix] Block user from changing pad after he/she is disconnected Use same approach of when channel state is chaged to "DISCONNECTED". --- src/static/js/pad.js | 44 ++++++++++++--------- tests/frontend/specs/automatic_reconnect.js | 13 ++++++ 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/src/static/js/pad.js b/src/static/js/pad.js index c967e4615..4fefadf38 100644 --- a/src/static/js/pad.js +++ b/src/static/js/pad.js @@ -1,5 +1,5 @@ /** - * This code is mostly from the old Etherpad. Please help us to comment this code. + * This code is mostly from the old Etherpad. Please help us to comment this code. * This helps other people to understand this code better and helps them to improve it. * TL;DR COMMENTS ON THIS FILE ARE HIGHLY APPRECIATED */ @@ -99,15 +99,15 @@ function getParams() setting.callback(value); } } - + // Then URL applied stuff var params = getUrlVars() - + for(var i = 0; i < getParameters.length; i++) { var setting = getParameters[i]; var value = params[setting.name]; - + if(value && (value == setting.checkVal || setting.checkVal == null)) { setting.callback(value); @@ -156,7 +156,7 @@ function sendClientReady(isReconnect, messageType) token = "t." + randomString(); createCookie("token", token, 60); } - + var sessionID = decodeURIComponent(readCookie("sessionID")); var password = readCookie("password"); @@ -169,14 +169,14 @@ function sendClientReady(isReconnect, messageType) "token": token, "protocolVersion": 2 }; - + //this is a reconnect, lets tell the server our revisionnumber if(isReconnect == true) { msg.client_rev=pad.collabClient.getCurrentRevisionNumber(); msg.reconnect=true; } - + socket.json.send(msg); } @@ -203,12 +203,12 @@ function handshake() socket.once('connect', function () { sendClientReady(false); }); - + socket.on('reconnect', function () { pad.collabClient.setChannelState("CONNECTED"); pad.sendClientReady(true); }); - + socket.on('reconnecting', function() { pad.collabClient.setChannelState("RECONNECTING"); }); @@ -254,7 +254,7 @@ function handshake() $("#passwordinput").focus(); } } - + //if we haven't recieved the clientVars yet, then this message should it be else if (!receivedClientVars && obj.type == "CLIENT_VARS") { @@ -267,7 +267,7 @@ function handshake() clientVars = obj.data; clientVars.userAgent = "Anonymous"; clientVars.collab_client_vars.clientAgent = "Anonymous"; - + //initalize the pad pad._afterHandshake(); initalized = true; @@ -298,7 +298,7 @@ function handshake() { pad.changeViewOption('noColors', true); } - + if (settings.rtlIsTrue == true) { pad.changeViewOption('rtlIsTrue', true); @@ -335,6 +335,12 @@ function handshake() console.warn(obj); padconnectionstatus.disconnected(obj.disconnect); socket.disconnect(); + + // block user from making any change to the pad + padeditor.disable(); + padeditbar.disable(); + padimpexp.disable(); + return; } else @@ -345,13 +351,13 @@ function handshake() }); // Bind the colorpicker var fb = $('#colorpicker').farbtastic({ callback: '#mycolorpickerpreview', width: 220}); - // Bind the read only button + // Bind the read only button $('#readonlyinput').on('click',function(){ padeditbar.setEmbedLinks(); }); } -$.extend($.gritter.options, { +$.extend($.gritter.options, { position: 'bottom-right', // defaults to 'top-right' but can be 'bottom-left', 'bottom-right', 'top-left', 'top-right' (added in 1.7.1) fade: false, // dont fade, too jerky on mobile time: 6000 // hang on the screen for... @@ -424,7 +430,7 @@ var pad = { if(window.history && window.history.pushState) { $('#chattext p').remove(); //clear the chat messages - window.history.pushState("", "", newHref); + window.history.pushState("", "", newHref); receivedClientVars = false; sendClientReady(false, 'SWITCH_TO_PAD'); } @@ -731,20 +737,20 @@ var pad = { pad.diagnosticInfo.disconnectedMessage = message; pad.diagnosticInfo.padId = pad.getPadId(); pad.diagnosticInfo.socket = {}; - - //we filter non objects from the socket object and put them in the diagnosticInfo + + //we filter non objects from the socket object and put them in the diagnosticInfo //this ensures we have no cyclic data - this allows us to stringify the data for(var i in socket.socket) { var value = socket.socket[i]; var type = typeof value; - + if(type == "string" || type == "number") { pad.diagnosticInfo.socket[i] = value; } } - + pad.asyncSendDiagnosticInfo(); if (typeof window.ajlog == "string") { diff --git a/tests/frontend/specs/automatic_reconnect.js b/tests/frontend/specs/automatic_reconnect.js index e2d2df36a..9e4783e75 100644 --- a/tests/frontend/specs/automatic_reconnect.js +++ b/tests/frontend/specs/automatic_reconnect.js @@ -33,6 +33,19 @@ describe('Automatic pad reload on Force Reconnect message', function() { done(); }); + it('disables editor', function(done) { + var editorDocument = helper.padOuter$("iframe[name='ace_inner']").get(0).contentDocument; + var editorBody = editorDocument.getElementById('innerdocbody'); + + var editorIsEditable = editorBody.contentEditable === 'false' // IE/Safari + || editorDocument.designMode === 'off'; // other browsers + + expect(editorIsEditable).to.be(true); + + done(); + }); + + context('and user clicks on Cancel', function() { beforeEach(function() { var $errorMessageModal = helper.padChrome$('#connectivity .userdup'); From 4eec3763b4110cbd7b436a0c6505055ee5ca2af5 Mon Sep 17 00:00:00 2001 From: Luiza Pagliari Date: Thu, 4 May 2017 11:22:18 -0300 Subject: [PATCH 2/4] [fix] Close modals when user clicks both on pad inner and outer Also: split tests for automatic reconnection and regular modal tests. --- src/static/js/ace2_inner.js | 9 ++- tests/frontend/specs/automatic_reconnect.js | 13 ----- tests/frontend/specs/pad_modal.js | 64 +++++++++++++++++++++ 3 files changed, 72 insertions(+), 14 deletions(-) create mode 100644 tests/frontend/specs/pad_modal.js diff --git a/src/static/js/ace2_inner.js b/src/static/js/ace2_inner.js index f44a6583a..b1aebf3cf 100644 --- a/src/static/js/ace2_inner.js +++ b/src/static/js/ace2_inner.js @@ -3367,7 +3367,12 @@ function Ace2Inner(){ evt.preventDefault(); } } - //hide the dropdownso + + hideEditBarDropdowns(); + } + + function hideEditBarDropdowns() + { if(window.parent.parent.padeditbar){ // required in case its in an iframe should probably use parent.. See Issue 327 https://github.com/ether/etherpad-lite/issues/327 window.parent.parent.padeditbar.toggleDropDown("none"); } @@ -4983,6 +4988,8 @@ function Ace2Inner(){ $(document).on("keypress", handleKeyEvent); $(document).on("keyup", handleKeyEvent); $(document).on("click", handleClick); + // dropdowns on edit bar need to be closed on clicks on both pad inner and pad outer + $(outerWin.document).on("click", hideEditBarDropdowns); // Disabled: https://github.com/ether/etherpad-lite/issues/2546 // Will break OL re-numbering: https://github.com/ether/etherpad-lite/pull/2533 // $(document).on("cut", handleCut); diff --git a/tests/frontend/specs/automatic_reconnect.js b/tests/frontend/specs/automatic_reconnect.js index 9e4783e75..e2d2df36a 100644 --- a/tests/frontend/specs/automatic_reconnect.js +++ b/tests/frontend/specs/automatic_reconnect.js @@ -33,19 +33,6 @@ describe('Automatic pad reload on Force Reconnect message', function() { done(); }); - it('disables editor', function(done) { - var editorDocument = helper.padOuter$("iframe[name='ace_inner']").get(0).contentDocument; - var editorBody = editorDocument.getElementById('innerdocbody'); - - var editorIsEditable = editorBody.contentEditable === 'false' // IE/Safari - || editorDocument.designMode === 'off'; // other browsers - - expect(editorIsEditable).to.be(true); - - done(); - }); - - context('and user clicks on Cancel', function() { beforeEach(function() { var $errorMessageModal = helper.padChrome$('#connectivity .userdup'); diff --git a/tests/frontend/specs/pad_modal.js b/tests/frontend/specs/pad_modal.js new file mode 100644 index 000000000..d3afe107e --- /dev/null +++ b/tests/frontend/specs/pad_modal.js @@ -0,0 +1,64 @@ +describe('Pad modal', function() { + var padId, $originalPadFrame; + + beforeEach(function(done) { + padId = helper.newPad(function() { + // open same pad on another iframe, to force userdup error + var $otherIframeWithSamePad = $(''); + $originalPadFrame = $('#iframe-container iframe'); + $otherIframeWithSamePad.insertAfter($originalPadFrame); + + // wait for modal to be displayed + var $errorMessageModal = helper.padChrome$('#connectivity .userdup'); + helper.waitFor(function() { + return $errorMessageModal.is(':visible'); + }, 50000).done(done); + }); + + this.timeout(60000); + }); + + it('disables editor', function(done) { + var editorDocument = helper.padOuter$("iframe[name='ace_inner']").get(0).contentDocument; + var editorBody = editorDocument.getElementById('innerdocbody'); + + var editorIsEditable = editorBody.contentEditable === 'false' // IE/Safari + || editorDocument.designMode === 'off'; // other browsers + + expect(editorIsEditable).to.be(true); + + done(); + }); + + context('and user clicks on editor', function() { + beforeEach(function() { + var $editor = helper.padInner$('#innerdocbody'); + $editor.click(); + }); + + it('closes the modal', function(done) { + var $errorMessageModal = helper.padChrome$('#connectivity .userdup'); + var modalIsVisible = $errorMessageModal.is(':visible'); + + expect(modalIsVisible).to.be(false); + + done(); + }); + }); + + context('and user clicks on pad outer', function() { + beforeEach(function() { + var $lineNumbersColumn = helper.padOuter$('#sidedivinner'); + $lineNumbersColumn.click(); + }); + + it('closes the modal', function(done) { + var $errorMessageModal = helper.padChrome$('#connectivity .userdup'); + var modalIsVisible = $errorMessageModal.is(':visible'); + + expect(modalIsVisible).to.be(false); + + done(); + }); + }); +}); From 9176bf9bad8d30828ec7cf215357d5a054842c57 Mon Sep 17 00:00:00 2001 From: Luiza Pagliari Date: Thu, 4 May 2017 14:34:01 -0300 Subject: [PATCH 3/4] [fix] Do not close "force reconnect" messages If a "force reconnect" message is displayed to the user, it means the only way to go back to a healthy state is to reload the pad. So we cannot hide this kind of message, like what is done with other modals (eg: "settings"). --- src/static/js/pad_editbar.js | 24 +++-- tests/frontend/specs/pad_modal.js | 171 +++++++++++++++++++++--------- 2 files changed, 137 insertions(+), 58 deletions(-) diff --git a/src/static/js/pad_editbar.js b/src/static/js/pad_editbar.js index dd1c377a3..b2aade466 100644 --- a/src/static/js/pad_editbar.js +++ b/src/static/js/pad_editbar.js @@ -259,18 +259,25 @@ var padeditbar = (function() // hide all modules and remove highlighting of all buttons if(moduleName == "none") { - var returned = false + var returned = false; for(var i=0;i a").removeClass("selected"); + $("li[data-key=" + thisModuleName + "] > a").removeClass("selected"); module.slideUp("fast", cb); returned = true; } @@ -283,16 +290,17 @@ var padeditbar = (function() // respectively add highlighting to the corresponding button for(var i=0;i a").removeClass("selected"); + $("li[data-key=" + thisModuleName + "] > a").removeClass("selected"); module.slideUp("fast"); } - else if(self.dropdowns[i]==moduleName) + else if(thisModuleName==moduleName) { - $("li[data-key=" + self.dropdowns[i] + "] > a").addClass("selected"); + $("li[data-key=" + thisModuleName + "] > a").addClass("selected"); module.slideDown("fast", cb); } } diff --git a/tests/frontend/specs/pad_modal.js b/tests/frontend/specs/pad_modal.js index d3afe107e..15eb8ac86 100644 --- a/tests/frontend/specs/pad_modal.js +++ b/tests/frontend/specs/pad_modal.js @@ -1,64 +1,135 @@ describe('Pad modal', function() { - var padId, $originalPadFrame; + context('when modal is a "force reconnect" message', function() { + var MODAL_SELECTOR = '#connectivity .userdup'; - beforeEach(function(done) { - padId = helper.newPad(function() { - // open same pad on another iframe, to force userdup error - var $otherIframeWithSamePad = $(''); - $originalPadFrame = $('#iframe-container iframe'); - $otherIframeWithSamePad.insertAfter($originalPadFrame); + var padId, $originalPadFrame; - // wait for modal to be displayed - var $errorMessageModal = helper.padChrome$('#connectivity .userdup'); - helper.waitFor(function() { - return $errorMessageModal.is(':visible'); - }, 50000).done(done); + beforeEach(function(done) { + padId = helper.newPad(function() { + // open same pad on another iframe, to force userdup error + var $otherIframeWithSamePad = $(''); + $originalPadFrame = $('#iframe-container iframe'); + $otherIframeWithSamePad.insertAfter($originalPadFrame); + + // wait for modal to be displayed + var $modal = helper.padChrome$(MODAL_SELECTOR); + helper.waitFor(function() { + return $modal.is(':visible'); + }, 50000).done(done); + }); + + this.timeout(60000); }); - this.timeout(60000); + it('disables editor', function(done) { + expect(isEditorDisabled()).to.be(true); + + done(); + }); + + context('and user clicks on editor', function() { + beforeEach(function() { + clickOnPadInner(); + }); + + it('does not close the modal', function(done) { + var $modal = helper.padChrome$(MODAL_SELECTOR); + var modalIsVisible = $modal.is(':visible'); + + expect(modalIsVisible).to.be(true); + + done(); + }); + }); + + context('and user clicks on pad outer', function() { + beforeEach(function() { + clickOnPadOuter(); + }); + + it('does not close the modal', function(done) { + var $modal = helper.padChrome$(MODAL_SELECTOR); + var modalIsVisible = $modal.is(':visible'); + + expect(modalIsVisible).to.be(true); + + done(); + }); + }); }); - it('disables editor', function(done) { + // we use "settings" here, but other modals have the same behaviour + context('when modal is not an error message', function() { + var MODAL_SELECTOR = '#settings'; + + beforeEach(function(done) { + helper.newPad(function() { + openSettingsAndWaitForModalToBeVisible(done); + }); + + this.timeout(60000); + }); + + it('does not disable editor', function(done) { + expect(isEditorDisabled()).to.be(false); + done(); + }); + + context('and user clicks on editor', function() { + beforeEach(function() { + clickOnPadInner(); + }); + + it('closes the modal', function(done) { + expect(isModalOpened(MODAL_SELECTOR)).to.be(false); + done(); + }); + }); + + context('and user clicks on pad outer', function() { + beforeEach(function() { + clickOnPadOuter(); + }); + + it('closes the modal', function(done) { + expect(isModalOpened(MODAL_SELECTOR)).to.be(false); + done(); + }); + }); + }); + + var clickOnPadInner = function() { + var $editor = helper.padInner$('#innerdocbody'); + $editor.click(); + } + + var clickOnPadOuter = function() { + var $lineNumbersColumn = helper.padOuter$('#sidedivinner'); + $lineNumbersColumn.click(); + } + + var openSettingsAndWaitForModalToBeVisible = function(done) { + helper.padChrome$('.buttonicon-settings').click(); + + // wait for modal to be displayed + var modalSelector = '#settings'; + helper.waitFor(function() { + return isModalOpened(modalSelector); + }, 10000).done(done); + } + + var isEditorDisabled = function() { var editorDocument = helper.padOuter$("iframe[name='ace_inner']").get(0).contentDocument; var editorBody = editorDocument.getElementById('innerdocbody'); - var editorIsEditable = editorBody.contentEditable === 'false' // IE/Safari + var editorIsDisabled = editorBody.contentEditable === 'false' // IE/Safari || editorDocument.designMode === 'off'; // other browsers - expect(editorIsEditable).to.be(true); + return editorIsDisabled; + } - done(); - }); - - context('and user clicks on editor', function() { - beforeEach(function() { - var $editor = helper.padInner$('#innerdocbody'); - $editor.click(); - }); - - it('closes the modal', function(done) { - var $errorMessageModal = helper.padChrome$('#connectivity .userdup'); - var modalIsVisible = $errorMessageModal.is(':visible'); - - expect(modalIsVisible).to.be(false); - - done(); - }); - }); - - context('and user clicks on pad outer', function() { - beforeEach(function() { - var $lineNumbersColumn = helper.padOuter$('#sidedivinner'); - $lineNumbersColumn.click(); - }); - - it('closes the modal', function(done) { - var $errorMessageModal = helper.padChrome$('#connectivity .userdup'); - var modalIsVisible = $errorMessageModal.is(':visible'); - - expect(modalIsVisible).to.be(false); - - done(); - }); - }); + var isModalOpened = function(modalSelector) { + var $modal = helper.padChrome$(modalSelector); + return $modal.is(':visible'); + } }); From 894ebffcaf3d5fcab957f5e40d1fc94359003d54 Mon Sep 17 00:00:00 2001 From: Luiza Pagliari Date: Fri, 12 May 2017 07:03:40 -0300 Subject: [PATCH 4/4] [fix] Do not close ANY "force reconnect" message Fix previous commit. As "force reconnect" buttons have all the same id on DOM, on the previous commit we were only disallowing the first button with that id on DOM -- "userdup" -- to be closed by a click on editor. Casually the tests were using the same error to simulate a "force reconnect", so even the tests were not getting the issue. --- src/static/js/pad_editbar.js | 2 +- tests/frontend/specs/pad_modal.js | 12 ++++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/static/js/pad_editbar.js b/src/static/js/pad_editbar.js index b2aade466..9cf357aad 100644 --- a/src/static/js/pad_editbar.js +++ b/src/static/js/pad_editbar.js @@ -271,7 +271,7 @@ var padeditbar = (function() var module = $("#" + thisModuleName); //skip any "force reconnect" message - var isAForceReconnectMessage = module.find('#forcereconnect').is(':visible'); + var isAForceReconnectMessage = module.find('button#forcereconnect:visible').length > 0; if(isAForceReconnectMessage) continue; diff --git a/tests/frontend/specs/pad_modal.js b/tests/frontend/specs/pad_modal.js index 15eb8ac86..80752e4b8 100644 --- a/tests/frontend/specs/pad_modal.js +++ b/tests/frontend/specs/pad_modal.js @@ -1,15 +1,11 @@ describe('Pad modal', function() { context('when modal is a "force reconnect" message', function() { - var MODAL_SELECTOR = '#connectivity .userdup'; - - var padId, $originalPadFrame; + var MODAL_SELECTOR = '#connectivity .slowcommit'; beforeEach(function(done) { - padId = helper.newPad(function() { - // open same pad on another iframe, to force userdup error - var $otherIframeWithSamePad = $(''); - $originalPadFrame = $('#iframe-container iframe'); - $otherIframeWithSamePad.insertAfter($originalPadFrame); + helper.newPad(function() { + // force a "slowcommit" error + helper.padChrome$.window.pad.handleChannelStateChange('DISCONNECTED', 'slowcommit'); // wait for modal to be displayed var $modal = helper.padChrome$(MODAL_SELECTOR);