diff --git a/CHANGELOG.md b/CHANGELOG.md index 618a88e10..8069e84c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +# 1.8.3 +*BREAKING CHANGE*: undoing the "clear authorship colors" command is no longer supported (see https://github.com/ether/etherpad-lite/issues/2802) + # 1.8 * SECURITY: change referrer policy so that Etherpad addresses aren't leaked when links are clicked (discussion: https://github.com/ether/etherpad-lite/pull/3636) * SECURITY: set the "secure" flag for the session cookies when served over SSL. From now on it will not be possible to serve the same instance both in cleartext and over SSL diff --git a/src/locales/en.json b/src/locales/en.json index c8ef1a7ae..5a86a6618 100644 --- a/src/locales/en.json +++ b/src/locales/en.json @@ -154,7 +154,7 @@ "pad.userlist.guest": "Guest", "pad.userlist.deny": "Deny", "pad.userlist.approve": "Approve", - "pad.editbar.clearcolors": "Clear authorship colors on entire document?", + "pad.editbar.clearcolors": "Clear authorship colors on entire document? This cannot be undone", "pad.impexp.importbutton": "Import Now", "pad.impexp.importing": "Importing...", diff --git a/src/locales/qqq.json b/src/locales/qqq.json index 512ef6a31..de357fb73 100644 --- a/src/locales/qqq.json +++ b/src/locales/qqq.json @@ -113,7 +113,7 @@ "pad.userlist.guest": "Preceded by the link text which is labeled {{msg-etherpadlite|Pad.userlist.approve}}.\n{{Identical|Guest}}", "pad.userlist.deny": "Used as link text.\n\nFollowed by the link which is labeled {{msg-etherpadlite|Pad.userlist.approve}}.", "pad.userlist.approve": "Used as link text.\n\nPreceded by the link which is labeled {{msg-etherpadlite|Pad.userlist.deny}}.\n\nFollowed by the message {{msg-etherpadlite|Pad.userlist.guest}}.\n{{Identical|Approve}}", - "pad.editbar.clearcolors": "Used as confirmation message (JavaScript confirm() function).\n\nThis message means \"Are you sure you want to clear authorship colors on entire document?\".", + "pad.editbar.clearcolors": "Used as confirmation message (JavaScript confirm() function).\n\nThis message means \"Are you sure you want to clear authorship colors on entire document? This cannot be undone\".", "pad.impexp.importbutton": "Used as label for the Submit button.", "pad.impexp.importing": "Used to indicate that the file is being imported.\n{{Identical|Importing}}", "pad.impexp.confirmimport": "Used as confirmation message (JavaScript confirm() function).", diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js index 0e16f4b69..5be683e2d 100644 --- a/src/node/handler/PadMessageHandler.js +++ b/src/node/handler/PadMessageHandler.js @@ -932,7 +932,7 @@ async function handleClientReady(client, message) await Promise.all(authors.map(authorId => { return authorManager.getAuthor(authorId).then(author => { if (!author) { - messageLogger.error("There is no author for authorId:", authorId); + messageLogger.error("There is no author for authorId: ", authorId, ". This is possibly related to https://github.com/ether/etherpad-lite/issues/2802"); } else { historicalAuthorData[authorId] = { name: author.name, colorId: author.colorId }; // Filter author attribs (e.g. don't send author's pads to all clients) } diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js index 2458ae65e..e8d182653 100644 --- a/src/static/js/Changeset.js +++ b/src/static/js/Changeset.js @@ -791,6 +791,9 @@ exports.textLinesMutator = function (lines) { } } else { var sline = putCurLineInSplice(); + if (!curSplice[sline]) { + console.error("curSplice[sline] not populated, actual curSplice contents is ", curSplice, ". Possibly related to https://github.com/ether/etherpad-lite/issues/2802"); + } curSplice[sline] = curSplice[sline].substring(0, curCol) + text + curSplice[sline].substring(curCol); curCol += text.length; } diff --git a/src/static/js/ace2_inner.js b/src/static/js/ace2_inner.js index dd8e41f47..2bd93fcf7 100644 --- a/src/static/js/ace2_inner.js +++ b/src/static/js/ace2_inner.js @@ -292,6 +292,7 @@ function Ace2Inner(){ { if ((typeof author) != "string") { + top.console.error("Going to throw new error, potentially caused by: https://github.com/ether/etherpad-lite/issues/2802"); throw new Error("setAuthorInfo: author (" + author + ") is not a string"); } if (!info) diff --git a/src/static/js/undomodule.js b/src/static/js/undomodule.js index 6610224fe..7bb47a625 100644 --- a/src/static/js/undomodule.js +++ b/src/static/js/undomodule.js @@ -253,7 +253,15 @@ var undoModule = (function() } if (!merged) { - stack.pushEvent(event); + /* + * Push the event on the undo stack only if it exists, and if it's + * not a "clearauthorship". This disallows undoing the removal of the + * authorship colors, but is a necessary stopgap measure against + * https://github.com/ether/etherpad-lite/issues/2802 + */ + if (event && (event.eventType !== "clearauthorship")) { + stack.pushEvent(event); + } } undoPtr = 0; } diff --git a/tests/frontend/specs/clear_authorship_colors.js b/tests/frontend/specs/clear_authorship_colors.js index d38cbfe9b..efadb08a0 100644 --- a/tests/frontend/specs/clear_authorship_colors.js +++ b/tests/frontend/specs/clear_authorship_colors.js @@ -47,6 +47,76 @@ describe("clear authorship colors button", function(){ var hasAuthorClass = inner$("div").first().attr("class").indexOf("author") !== -1; expect(hasAuthorClass).to.be(false); + setTimeout(function(){ + var disconnectVisible = chrome$("div.disconnected").attr("class").indexOf("visible") === -1 + expect(disconnectVisible).to.be(true); + },1000); + + done(); + }); + + }); + + it("makes text clear authorship colors and checks it can't be undone", function(done) { + var inner$ = helper.padInner$; + var chrome$ = helper.padChrome$; + + // override the confirm dialogue functioon + helper.padChrome$.window.confirm = function(){ + return true; + } + + //get the first text element out of the inner iframe + var $firstTextElement = inner$("div").first(); + + // Get the original text + var originalText = inner$("div").first().text(); + + // Set some new text + var sentText = "Hello"; + + //select this text element + $firstTextElement.sendkeys('{selectall}'); + $firstTextElement.sendkeys(sentText); + $firstTextElement.sendkeys('{rightarrow}'); + + helper.waitFor(function(){ + return inner$("div span").first().attr("class").indexOf("author") !== -1; // wait until we have the full value available + }).done(function(){ + //IE hates you if you don't give focus to the inner frame bevore you do a clearAuthorship + inner$("div").first().focus(); + + //get the clear authorship colors button and click it + var $clearauthorshipcolorsButton = chrome$(".buttonicon-clearauthorship"); + $clearauthorshipcolorsButton.click(); + + // does the first divs span include an author class? + console.log(inner$("div span").first().attr("class")); + var hasAuthorClass = inner$("div span").first().attr("class").indexOf("author") !== -1; + //expect(hasAuthorClass).to.be(false); + + // does the first div include an author class? + var hasAuthorClass = inner$("div").first().attr("class").indexOf("author") !== -1; + expect(hasAuthorClass).to.be(false); + + var e = inner$.Event(helper.evtType); + e.ctrlKey = true; // Control key + e.which = 90; // z + inner$("#innerdocbody").trigger(e); // shouldn't od anything + + // does the first div include an author class? + hasAuthorClass = inner$("div").first().attr("class").indexOf("author") !== -1; + expect(hasAuthorClass).to.be(false); + + // get undo and redo buttons + var $undoButton = chrome$(".buttonicon-undo"); + + // click the button + $undoButton.click(); // shouldn't do anything + hasAuthorClass = inner$("div").first().attr("class").indexOf("author") !== -1; + expect(hasAuthorClass).to.be(false); + + setTimeout(function(){ var disconnectVisible = chrome$("div.disconnected").attr("class").indexOf("visible") === -1 expect(disconnectVisible).to.be(true); @@ -57,3 +127,4 @@ describe("clear authorship colors button", function(){ }); }); +