Merge branch 'develop' of github.com:ether/etherpad-lite into editbar-accessibility

8 files changed, 188 insertions, 107 deletions

diff --git a/README.md b/README.md
index 48ff434e..3d266827 100644
--- a/README.md
+++ b/README.md
@@ -46,6 +46,12 @@ Now, run `start.bat` and open <http://localhost:9001> in your browser.
 
 Update to the latest version with `git pull origin`, then run `bin\installOnWindows.bat`, again.
 
+If cloning to a subdirectory within another project, you may need to do the following:
+
+1. Start the server manually (e.g. `node/node_modules/ep_etherpad-lite/node/server.js]`)
+2. Edit the db `filename` in `settings.json` to the relative directory with the file (e.g. `application/lib/etherpad-lite/var/dirty.db`)
+3. Add auto-generated files to the main project `.gitignore`
+
 [Next steps](#next-steps).
 
 ## GNU/Linux and other UNIX-like systems
diff --git a/bin/safeRun.sh b/bin/safeRun.sh
index 4b3485ba..519a0b6e 100755
--- a/bin/safeRun.sh
+++ b/bin/safeRun.sh
@@ -55,7 +55,7 @@ do
     TIME_SINCE_LAST_SEND=$(($TIME_NOW - $LAST_EMAIL_SEND))
     
     if [ $TIME_SINCE_LAST_SEND -gt $TIME_BETWEEN_EMAILS ]; then
-      printf "Server was restared at: $(date)\nThe last 50 lines of the log before the error happens:\n $(tail -n 50 ${LOG})" | mail -s "Pad Server was restarted" $EMAIL_ADDRESS
+      printf "Server was restarted at: $(date)\nThe last 50 lines of the log before the error happens:\n $(tail -n 50 ${LOG})" | mail -s "Pad Server was restarted" $EMAIL_ADDRESS
       
       LAST_EMAIL_SEND=$TIME_NOW
     fi
diff --git a/doc/api/hooks_client-side.md b/doc/api/hooks_client-side.md
index 8e2d3da7..f9ad9147 100644
--- a/doc/api/hooks_client-side.md
+++ b/doc/api/hooks_client-side.md
@@ -203,6 +203,13 @@ Things in context:
 
 This hook is called before the content of a node is collected by the usual methods. The cc object can be used to do a bunch of things that modify the content of the pad. See, for example, the heading1 plugin for etherpad original.
 
+E.g. if you need to apply an attribute to newly inserted characters, 
+call cc.doAttrib(state, "attributeName") which results in an attribute attributeName=true.
+
+If you want to specify also a value, call cc.doAttrib(state, "attributeName:value")
+which results in an attribute attributeName=value.
+
+
 ## collectContentImage
 Called from: src/static/js/contentcollector.js
 
diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js
index 7ea5039d..c210ab2b 100644
--- a/src/node/handler/PadMessageHandler.js
+++ b/src/node/handler/PadMessageHandler.js
@@ -656,12 +656,17 @@ function handleUserChanges(data, cb)
           , op
         while(iterator.hasNext()) {
           op = iterator.next()
-          if(op.opcode != '+') continue;
+
+          //+ can add text with attribs
+          //= can change or add attribs
+          //- can have attribs, but they are discarded and don't show up in the attribs - but do show up in the  pool
+
           op.attribs.split('*').forEach(function(attr) {
             if(!attr) return
             attr = wireApool.getAttrib(attr)
             if(!attr) return
-            if('author' == attr[0] && attr[1] != thisSession.author) throw new Error("Trying to submit changes as another author in changeset "+changeset);
+            //the empty author is used in the clearAuthorship functionality so this should be the only exception
+            if('author' == attr[0] && (attr[1] != thisSession.author && attr[1] != '')) throw new Error("Trying to submit changes as another author in changeset "+changeset);
           })
         }
 
@@ -1629,10 +1634,15 @@ function composePadChangesets(padId, startNum, endNum, callback)
       changeset = changesets[startNum];
       var pool = pad.apool();
 
-      for(var r=startNum+1;r<endNum;r++)
-      {
-        var cs = changesets[r];
-        changeset = Changeset.compose(changeset, cs, pool);
+      try {
+        for(var r=startNum+1;r<endNum;r++) {
+          var cs = changesets[r];
+          changeset = Changeset.compose(changeset, cs, pool);
+        }
+      } catch(e){
+        // r-1 indicates the rev that was build starting with startNum, applying startNum+1, +2, +3
+        console.warn("failed to compose cs in pad:",padId," startrev:",startNum," current rev:",r);
+        return callback(e);
       }
 
       callback(null);
diff --git a/src/static/js/AttributeManager.js b/src/static/js/AttributeManager.js
index 865569c5..3f464908 100644
--- a/src/static/js/AttributeManager.js
+++ b/src/static/js/AttributeManager.js
@@ -98,7 +98,7 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
   
   /*
     Gets all attributes on a line
-    @param lineNum: the number of the line to set the attribute for 
+    @param lineNum: the number of the line to get the attribute for 
   */
   getAttributesOnLine: function(lineNum){
     // get attributes of first char of line
@@ -123,6 +123,59 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
   },
   
   /*
+    Gets all attributes at a position containing line number and column
+    @param lineNumber starting with zero
+    @param column starting with zero
+    returns a list of attributes in the format 
+    [ ["key","value"], ["key","value"], ...  ]
+  */
+  getAttributesOnPosition: function(lineNumber, column){
+    // get all attributes of the line
+    var aline = this.rep.alines[lineNumber];
+    
+    if (!aline) {
+        return [];
+    }
+    // iterate through all operations of a line
+    var opIter = Changeset.opIterator(aline);
+    
+    // we need to sum up how much characters each operations take until the wanted position
+    var currentPointer = 0;
+    var attributes = [];    
+    var currentOperation;
+    
+    while (opIter.hasNext()) {
+      currentOperation = opIter.next();
+      currentPointer = currentPointer + currentOperation.chars;      
+      
+      if (currentPointer > column) {
+        // we got the operation of the wanted position, now collect all its attributes
+        Changeset.eachAttribNumber(currentOperation.attribs, function (n) {
+          attributes.push([
+            this.rep.apool.getAttribKey(n),
+            this.rep.apool.getAttribValue(n)
+          ]);
+        }.bind(this));
+        
+        // skip the loop
+        return attributes;
+      }
+    }
+    return attributes;
+    
+  },
+  
+  /*
+    Gets all attributes at caret position 
+    if the user selected a range, the start of the selection is taken
+    returns a list of attributes in the format 
+    [ ["key","value"], ["key","value"], ...  ]
+  */
+  getAttributesOnCaret: function(){
+    return this.getAttributesOnPosition(this.rep.selStart[0], this.rep.selStart[1]);
+  },
+  
+  /*
     Sets a specified attribute on a line
     @param lineNum: the number of the line to set the attribute for
     @param attributeKey: the name of the attribute to set, e.g. list
@@ -153,40 +206,43 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
     return this.applyChangeset(builder);
   },
   
-  /*
-     Removes a specified attribute on a line
-     @param lineNum: the number of the affected line
-     @param attributeKey: the name of the attribute to remove, e.g. list
-
+ /**
+   * Removes a specified attribute on a line
+   *  @param lineNum the number of the affected line
+   *  @param attributeName the name of the attribute to remove, e.g. list
+   *  @param attributeValue if given only attributes with equal value will be removed
    */
-   removeAttributeOnLine: function(lineNum, attributeName, attributeValue){
-     var loc = [0,0];
-     var builder = Changeset.builder(this.rep.lines.totalWidth());
-     var hasMarker = this.lineHasMarker(lineNum);
-     var attribs
-     var foundAttrib = false
-     
-     attribs = this.getAttributesOnLine(lineNum).map(function(attrib) {
-       if(attrib[0] === attributeName) {
-         foundAttrib = true
-         return [attributeName, null] // remove this attrib from the linemarker
-       }
-       return attrib
-     })
+ removeAttributeOnLine: function(lineNum, attributeName, attributeValue){
+   var builder = Changeset.builder(this.rep.lines.totalWidth());
+   var hasMarker = this.lineHasMarker(lineNum);
+   var found = false;
 
-     if(!foundAttrib) {
-       return
+   var attribs = _(this.getAttributesOnLine(lineNum)).map(function (attrib) {
+     if (attrib[0] === attributeName && (!attributeValue || attrib[0] === attributeValue)){
+       found = true;
+       return [attributeName, ''];
      }
+     return attrib;
+   });
 
-     if(hasMarker){
-       ChangesetUtils.buildKeepRange(this.rep, builder, loc, (loc = [lineNum, 0]));
-       // If length == 4, there's [author, lmkr, insertorder, + the attrib being removed] thus we can remove the marker entirely
-       if(attribs.length <= 4) ChangesetUtils.buildRemoveRange(this.rep, builder, loc, (loc = [lineNum, 1]))
-       else ChangesetUtils.buildKeepRange(this.rep, builder, loc, (loc = [lineNum, 1]), attribs, this.rep.apool);
-     }
-     
-     return this.applyChangeset(builder);
-   },
+   if (!found) {
+     return;
+   }
+
+   ChangesetUtils.buildKeepToStartOfRange(this.rep, builder, [lineNum, 0]);
+
+   var countAttribsWithMarker = _.chain(attribs).filter(function(a){return !!a[1];})
+     .map(function(a){return a[0];}).difference(['author', 'lmkr', 'insertorder', 'start']).size().value();
+
+   //if we have marker and any of attributes don't need to have marker. we need delete it
+   if(hasMarker && !countAttribsWithMarker){
+     ChangesetUtils.buildRemoveRange(this.rep, builder, [lineNum, 0], [lineNum, 1]);
+   }else{
+     ChangesetUtils.buildKeepRange(this.rep, builder, [lineNum, 0], [lineNum, 1], attribs, this.rep.apool);
+   }
+
+   return this.applyChangeset(builder);
+ },
   
    /*
      Toggles a line attribute for the specified line number
@@ -204,4 +260,4 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
   }
 });
 
-module.exports = AttributeManager;
\ No newline at end of file
+module.exports = AttributeManager;
diff --git a/src/static/js/ace2_inner.js b/src/static/js/ace2_inner.js
index 3c6c7cb8..eef99b1b 100644
--- a/src/static/js/ace2_inner.js
+++ b/src/static/js/ace2_inner.js
@@ -2322,93 +2322,72 @@ function Ace2Inner(){
   }
   editorInfo.ace_setAttributeOnSelection = setAttributeOnSelection;
 
-  function getAttributeOnSelection(attributeName){
-    if (!(rep.selStart && rep.selEnd)) return;
-
-    // get the previous/next characters formatting when we have nothing selected
-    // To fix this we just change the focus area, we don't actually check anything yet.
-    if(rep.selStart[1] == rep.selEnd[1]){
-      // if we're at the beginning of a line bump end forward so we get the right attribute
-      if(rep.selStart[1] == 0 && rep.selEnd[1] == 0){
-        rep.selEnd[1] = 1;
-      }
-      if(rep.selStart[1] < 0){
-        rep.selStart[1] = 0;
-      }
-      var line = rep.lines.atIndex(rep.selStart[0]);
-      // if we're at the end of the line bmp the start back 1 so we get hte attribute
-      if(rep.selEnd[1] == line.text.length){
-        rep.selStart[1] = rep.selStart[1] -1;
-      }
-    }
 
-    // Do the detection
-    var selectionAllHasIt = true;
+  function getAttributeOnSelection(attributeName){
+    if (!(rep.selStart && rep.selEnd)) return
+    
     var withIt = Changeset.makeAttribsString('+', [
       [attributeName, 'true']
     ], rep.apool);
     var withItRegex = new RegExp(withIt.replace(/\*/g, '\\*') + "(\\*|$)");
-
     function hasIt(attribs)
     {
       return withItRegex.test(attribs);
     }
 
-    var selStartLine = rep.selStart[0];
-    var selEndLine = rep.selEnd[0];
-    for (var n = selStartLine; n <= selEndLine; n++)
-    {
-      var opIter = Changeset.opIterator(rep.alines[n]);
-      var indexIntoLine = 0;
-      var selectionStartInLine = 0;
-      var selectionEndInLine = rep.lines.atIndex(n).text.length; // exclude newline
-      if(rep.lines.atIndex(n).text.length == 0){
-        return false; // If the line length is 0 we basically treat it as having no formatting
-      }
-      if(rep.selStart[1] == rep.selEnd[1] && rep.selStart[1] == rep.lines.atIndex(n).text.length){
-        return false; // If we're at the end of a line we treat it as having no formatting
-      }
-      if(rep.selStart[1] == 0 && rep.selEnd[1] == 0){
-        rep.selEnd[1] == 1;
-      }
-      if(rep.selEnd[1] == -1){
-        rep.selEnd[1] = 1; // sometimes rep.selEnd is -1, not sure why..  When it is we should look at the first char
-      }
-      if (n == selStartLine)
-      {
-        selectionStartInLine = rep.selStart[1];
-      }
-      if (n == selEndLine)
-      {
-        selectionEndInLine = rep.selEnd[1];
-      }
-      while (opIter.hasNext())
-      {
+    return rangeHasAttrib(rep.selStart, rep.selEnd)
+    
+    function rangeHasAttrib(selStart, selEnd) {
+      // if range is collapsed -> no attribs in range
+      if(selStart[1] == selEnd[1] && selStart[0] == selEnd[0]) return false
+      
+      if(selStart[0] != selEnd[0]) { // -> More than one line selected
+        var hasAttrib = true
+        
+        // from selStart to the end of the first line
+        hasAttrib = hasAttrib && rangeHasAttrib(selStart, [selStart[0], rep.lines.atIndex(selStart[0]).text.length])
+
+        // for all lines in between
+        for(var n=selStart[0]+1; n < selEnd[0]; n++) {
+          hasAttrib = hasAttrib && rangeHasAttrib([n, 0], [n, rep.lines.atIndex(n).text.length])
+        }
+
+        // for the last, potentially partial, line
+        hasAttrib = hasAttrib && rangeHasAttrib([selEnd[0], 0], [selEnd[0], selEnd[1]])
+        
+        return hasAttrib
+      }
+      
+      // Logic tells us we now have a range on a single line
+      
+      var lineNum = selStart[0]
+        , start = selStart[1]
+        , end = selEnd[1]
+        , hasAttrib = true
+      
+      // Iterate over attribs on this line
+      
+      var opIter = Changeset.opIterator(rep.alines[lineNum])
+        , indexIntoLine = 0
+      
+      while (opIter.hasNext()) {
         var op = opIter.next();
         var opStartInLine = indexIntoLine;
         var opEndInLine = opStartInLine + op.chars;
-        if (!hasIt(op.attribs))
-        {
+        if (!hasIt(op.attribs)) {
           // does op overlap selection?
-          if (!(opEndInLine <= selectionStartInLine || opStartInLine >= selectionEndInLine))
-          {
-            selectionAllHasIt = false;
+          if (!(opEndInLine <= start || opStartInLine >= end)) {
+            hasAttrib = false; // since it's overlapping but hasn't got the attrib -> range hasn't got it
             break;
           }
         }
         indexIntoLine = opEndInLine;
       }
-      if (!selectionAllHasIt)
-      {
-        break;
-      }
-    }
-    if(selectionAllHasIt){
-      return true;
-    }else{
-      return false;
+      
+      return hasAttrib
     }
   }
+  
   editorInfo.ace_getAttributeOnSelection = getAttributeOnSelection;
 
   function toggleAttributeOnSelection(attributeName)
diff --git a/src/static/js/contentcollector.js b/src/static/js/contentcollector.js
index e428c63f..857e171f 100644
--- a/src/static/js/contentcollector.js
+++ b/src/static/js/contentcollector.js
@@ -297,7 +297,23 @@ function makeContentCollector(collectStyles, abrowser, apool, domInterface, clas
     {
       if (state.attribs[a])
       {
-        lst.push([a, 'true']);
+        // The following splitting of the attribute name is a workaround
+        // to enable the content collector to store key-value attributes
+        // see https://github.com/ether/etherpad-lite/issues/2567 for more information
+        // in long term the contentcollector should be refactored to get rid of this workaround
+        var ATTRIBUTE_SPLIT_STRING = "::";
+        
+        // see if attributeString is splittable
+        var attributeSplits = a.split(ATTRIBUTE_SPLIT_STRING);
+        if (attributeSplits.length > 1) {
+            // the attribute name follows the convention key::value
+            // so save it as a key value attribute
+            lst.push([attributeSplits[0], attributeSplits[1]]);
+        } else {
+            // the "normal" case, the attribute is just a switch
+            // so set it true
+            lst.push([a, 'true']);
+        }
       }
     }
     if (state.authorLevel > 0)
@@ -571,7 +587,9 @@ function makeContentCollector(collectStyles, abrowser, apool, domInterface, clas
           }
           else if ((tname == "div" || tname == "p") && cls && cls.match(/(?:^| )ace-line\b/))
           {
-            oldListTypeOrNull = (_enterList(state, type) || 'none');
+            // This has undesirable behavior in Chrome but is right in other browsers.
+            // See https://github.com/ether/etherpad-lite/issues/2412 for reasoning
+            if(!abrowser.chrome) oldListTypeOrNull = (_enterList(state, type) || 'none');
           }
           if (className2Author && cls)
           {
diff --git a/tests/frontend/specs/clear_authorship_colors.js b/tests/frontend/specs/clear_authorship_colors.js
index 5db35612..1417f63c 100644
--- a/tests/frontend/specs/clear_authorship_colors.js
+++ b/tests/frontend/specs/clear_authorship_colors.js
@@ -47,6 +47,11 @@ 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();
     });