From c361df52d236fab92b9ba9ae68c5ad2cd8eeb84e Mon Sep 17 00:00:00 2001
From: John McLear <john@mclear.co.uk>
Date: Fri, 24 Sep 2021 16:46:53 +0100
Subject: [PATCH] bugfix: Allow selection to start/end before line marker

---
 src/static/js/AttributeManager.js             |  8 +++---
 .../frontend/specs/clear_authorship_colors.js | 26 ++++++++++++++++++-
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/src/static/js/AttributeManager.js b/src/static/js/AttributeManager.js
index a2ea15b6c..124434031 100644
--- a/src/static/js/AttributeManager.js
+++ b/src/static/js/AttributeManager.js
@@ -103,12 +103,12 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
     const markerWidth = this.lineHasMarker(row) ? 1 : 0;
     if (lineLength - markerWidth < 0) throw new Error(`line ${row} has negative length`);
 
-    const startCol = row === start[0] ? start[1] : markerWidth;
-    if (startCol - markerWidth < 0) throw new RangeError('selection starts before line start');
+    if (start[1] < 0) throw new RangeError('selection starts at negative column');
+    const startCol = Math.max(markerWidth, row === start[0] ? start[1] : 0);
     if (startCol > lineLength) throw new RangeError('selection starts after line end');
 
-    const endCol = row === end[0] ? end[1] : lineLength;
-    if (endCol - markerWidth < 0) throw new RangeError('selection ends before line start');
+    if (end[1] < 0) throw new RangeError('selection ends at negative column');
+    const endCol = Math.max(markerWidth, row === end[0] ? end[1] : lineLength);
     if (endCol > lineLength) throw new RangeError('selection ends after line end');
     if (startCol > endCol) throw new RangeError('selection ends before it starts');
 
diff --git a/src/tests/frontend/specs/clear_authorship_colors.js b/src/tests/frontend/specs/clear_authorship_colors.js
index 58ce93704..20c358b49 100644
--- a/src/tests/frontend/specs/clear_authorship_colors.js
+++ b/src/tests/frontend/specs/clear_authorship_colors.js
@@ -1,9 +1,11 @@
 'use strict';
 
 describe('clear authorship colors button', function () {
+  let padId;
+
   // create a new pad before each test run
   beforeEach(async function () {
-    await helper.aNewPad();
+    padId = await helper.aNewPad();
   });
 
   it('makes text clear authorship colors', async function () {
@@ -98,4 +100,26 @@ describe('clear authorship colors button', function () {
     await helper.waitForPromise(
         () => chrome$('div.disconnected').attr('class').indexOf('visible') === -1);
   });
+
+  // Test for https://github.com/ether/etherpad-lite/issues/5128
+  it('clears authorship when first line has line attributes', async function () {
+    // override the confirm dialogue function
+    helper.padChrome$.window.confirm = () => true;
+
+    // Make sure there is text with author info. The first line must have a line attribute.
+    await helper.clearPad();
+    await helper.edit('Hello');
+    helper.padChrome$('.buttonicon-insertunorderedlist').click();
+    await helper.waitForPromise(() => helper.padInner$('[class*="author-"]').length > 0);
+
+    const nCommits = helper.commits.length;
+    helper.padChrome$('.buttonicon-clearauthorship').click();
+    await helper.waitForPromise(() => helper.padInner$('[class*="author-"]').length === 0);
+
+    // Make sure the change was actually accepted by reloading the pad and looking for authorship.
+    // Before the pad can be reloaded the server might need some time to accept the change.
+    await helper.waitForPromise(() => helper.commits.length > nCommits);
+    await helper.aNewPad({id: padId});
+    expect(helper.padInner$('[class*="author-"]').length).to.be(0);
+  });
 });