Changeset: Migrate to the new attribute API

CHANGELOG.md

@@ -8,6 +8,18 @@
   (low-level API) and `ep_etherpad-lite/static/js/AttributeMap` (high-level
   API).
 
+### Compatibility changes
+
+#### For plugin authors
+
+* Changes to the `src/static/js/Changeset.js` library:
+  * The following attribute processing functions are deprecated (use the new
+    attribute APIs instead):
+    * `attribsAttributeValue()`
+    * `eachAttribNumber()`
+    * `makeAttribsString()`
+    * `opAttributeValue()`
+
 # 1.8.15
 
 ### Security fixes

doc/api/hooks_server-side.md

@@ -665,6 +665,7 @@ Context properties:
 Example:
 
 ```javascript
+const AttributeMap = require('ep_etherpad-lite/static/js/AttributeMap');
 const Changeset = require('ep_etherpad-lite/static/js/Changeset');
 
 exports.getLineHTMLForExport = async (hookName, context) => {
@@ -672,7 +673,7 @@ exports.getLineHTMLForExport = async (hookName, context) => {
   const opIter = Changeset.opIterator(context.attribLine);
   if (!opIter.hasNext()) return;
   const op = opIter.next();
-  const heading = Changeset.opAttributeValue(op, 'heading', apool);
+  const heading = AttributeMap.fromString(op.attribs, context.apool).get('heading');
   if (!heading) return;
   context.lineContent = `<${heading}>${context.lineContent}</${heading}>`;
 };

src/node/handler/PadMessageHandler.js

@@ -19,6 +19,7 @@
  * limitations under the License.
  */
 
+const AttributeMap = require('../../static/js/AttributeMap');
 const padManager = require('../db/PadManager');
 const Changeset = require('../../static/js/Changeset');
 const ChatMessage = require('../../static/js/ChatMessage');
@@ -32,7 +33,6 @@ const plugins = require('../../static/js/pluginfw/plugin_defs.js');
 const log4js = require('log4js');
 const messageLogger = log4js.getLogger('message');
 const accessLogger = log4js.getLogger('access');
-const _ = require('underscore');
 const hooks = require('../../static/js/pluginfw/hooks.js');
 const stats = require('../stats');
 const assert = require('assert').strict;
@@ -585,14 +585,6 @@ const handleUserChanges = async (socket, message) => {
     // Verify that the changeset has valid syntax and is in canonical form
     Changeset.checkRep(changeset);
 
-    // Verify that the attribute indexes used in the changeset are all
-    // defined in the accompanying attribute pool.
-    Changeset.eachAttribNumber(changeset, (n) => {
-      if (!wireApool.getAttrib(n)) {
-        throw new Error(`Attribute pool is missing attribute ${n} for changeset ${changeset}`);
-      }
-    });
-
     // Validate all added 'author' attribs to be the same value as the current user
     const iterator = Changeset.opIterator(Changeset.unpack(changeset).ops);
     let op;
@@ -605,19 +597,14 @@ const handleUserChanges = async (socket, message) => {
       // - 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((attr) => {
+      // Besides verifying the author attribute, this serves a second purpose:
-        if (!attr) return;
+      // AttributeMap.fromString() ensures that all attribute numbers are valid (it will throw if
-
+      // an attribute number isn't in the pool).
-        attr = wireApool.getAttrib(Changeset.parseNum(attr));
+      const opAuthorId = AttributeMap.fromString(op.attribs, wireApool).get('author');
-        if (!attr) return;
+      if (opAuthorId && opAuthorId !== thisSession.author) {
-
+        throw new Error(`Author ${thisSession.author} tried to submit changes as author ` +
-        // the empty author is used in the clearAuthorship functionality so this
+                        `${opAuthorId} in changeset ${changeset}`);
-        // should be the only exception
+      }
-        if ('author' === attr[0] && (attr[1] !== thisSession.author && attr[1] !== '')) {
-          throw new Error(`Author ${thisSession.author} tried to submit changes as author ` +
-                          `${attr[1]} in changeset ${changeset}`);
-        }
-      });
     }
 
     // ex. adoptChangesetAttribs
@@ -758,11 +745,8 @@ const _correctMarkersInPad = (atext, apool) => {
   let offset = 0;
   while (iter.hasNext()) {
     const op = iter.next();
-
+    const attribs = AttributeMap.fromString(op.attribs, apool);
-    const hasMarker = _.find(
+    const hasMarker = AttributeManager.lineAttributes.some((a) => attribs.has(a));
-        AttributeManager.lineAttributes,
-        (attribute) => Changeset.opAttributeValue(op, attribute, apool)) !== undefined;
-
     if (hasMarker) {
       for (let i = 0; i < op.chars; i++) {
         if (offset > 0 && text.charAt(offset - 1) !== '\n') {

src/node/utils/ExportHelper.js

@@ -19,6 +19,7 @@
  * limitations under the License.
  */
 
+const AttributeMap = require('../../static/js/AttributeMap');
 const Changeset = require('../../static/js/Changeset');
 
 exports.getPadPlainText = (pad, revNum) => {
@@ -54,7 +55,8 @@ exports._analyzeLine = (text, aline, apool) => {
     const opIter = Changeset.opIterator(aline);
     if (opIter.hasNext()) {
       const op = opIter.next();
-      let listType = Changeset.opAttributeValue(op, 'list', apool);
+      const attribs = AttributeMap.fromString(op.attribs, apool);
+      let listType = attribs.get('list');
       if (listType) {
         lineMarker = 1;
         listType = /([a-z]+)([0-9]+)/.exec(listType);
@@ -63,7 +65,7 @@ exports._analyzeLine = (text, aline, apool) => {
           line.listLevel = Number(listType[2]);
         }
       }
-      const start = Changeset.opAttributeValue(op, 'start', apool);
+      const start = attribs.get('start');
       if (start) {
         line.start = start;
       }

src/node/utils/ExportHtml.js

@@ -16,6 +16,7 @@
  */
 
 const Changeset = require('../../static/js/Changeset');
+const attributes = require('../../static/js/attributes');
 const padManager = require('../db/PadManager');
 const _ = require('underscore');
 const Security = require('../../static/js/security');
@@ -206,11 +207,11 @@ const getHTMLFromAtext = async (pad, atext, authorColors) => {
         const usedAttribs = [];
 
         // mark all attribs as used
-        Changeset.eachAttribNumber(o.attribs, (a) => {
+        for (const a of attributes.decodeAttribString(o.attribs)) {
           if (a in anumMap) {
             usedAttribs.push(anumMap[a]); // i = 0 => bold, etc.
           }
-        });
+        }
         let outermostTag = -1;
         // find the outer most open tag that is no longer used
         for (let i = openTags.length - 1; i >= 0; i--) {

src/node/utils/ExportTxt.js

@@ -20,6 +20,7 @@
  */
 
 const Changeset = require('../../static/js/Changeset');
+const attributes = require('../../static/js/attributes');
 const padManager = require('../db/PadManager');
 const _analyzeLine = require('./ExportHelper')._analyzeLine;
 
@@ -82,7 +83,7 @@ const getTXTFromAtext = (pad, atext, authorColors) => {
         const o = iter.next();
         let propChanged = false;
 
-        Changeset.eachAttribNumber(o.attribs, (a) => {
+        for (const a of attributes.decodeAttribString(o.attribs)) {
           if (a in anumMap) {
             const i = anumMap[a]; // i = 0 => bold, etc.
 
@@ -93,7 +94,7 @@ const getTXTFromAtext = (pad, atext, authorColors) => {
               propVals[i] = STAY;
             }
           }
-        });
+        }
 
         for (let i = 0; i < propVals.length; i++) {
           if (propVals[i] === true) {

src/node/utils/padDiff.js

@@ -1,5 +1,8 @@
 'use strict';
+
+const AttributeMap = require('../../static/js/AttributeMap');
 const Changeset = require('../../static/js/Changeset');
+const attributes = require('../../static/js/attributes');
 const exportHtml = require('./ExportHtml');
 
 function PadDiff(pad, fromRev, toRev) {
@@ -54,17 +57,11 @@ PadDiff.prototype._isClearAuthorship = function (changeset) {
     return false;
   }
 
-  const attributes = [];
+  const [appliedAttribute, anotherAttribute] =
-  Changeset.eachAttribNumber(changeset, (attrNum) => {
+      attributes.attribsFromString(clearOperator.attribs, this._pad.pool);
-    attributes.push(attrNum);
-  });
 
-  // check that this changeset uses only one attribute
+  // Check that the operation has exactly one attribute.
-  if (attributes.length !== 1) {
+  if (appliedAttribute == null || anotherAttribute != null) return false;
-    return false;
-  }
-
-  const appliedAttribute = this._pad.pool.getAttrib(attributes[0]);
 
   // check if the applied attribute is an anonymous author attribute
   if (appliedAttribute[0] !== 'author' || appliedAttribute[1] !== '') {
@@ -376,27 +373,19 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) {
       // If the text this operator applies to is only a star,
       // than this is a false positive and should be ignored
       if (csOp.attribs && textBank !== '*') {
-        const deletedAttrib = apool.putAttrib(['removed', true]);
+        const attribs = AttributeMap.fromString(csOp.attribs, apool);
-        let authorAttrib = apool.putAttrib(['author', '']);
-        const attribs = [];
-        Changeset.eachAttribNumber(csOp.attribs, (n) => {
-          const attrib = apool.getAttrib(n);
-          attribs.push(attrib);
-          if (attrib[0] === 'author') authorAttrib = n;
-        });
         const undoBackToAttribs = cachedStrFunc((oldAttribsStr) => {
-          const backAttribs = [];
+          const oldAttribs = AttributeMap.fromString(oldAttribsStr, apool);
+          const backAttribs = new AttributeMap(apool)
+              .set('author', '')
+              .set('removed', 'true');
           for (const [key, value] of attribs) {
-            const oldValue = Changeset.attribsAttributeValue(oldAttribsStr, key, apool);
+            const oldValue = oldAttribs.get(key);
-            if (oldValue !== value) backAttribs.push([key, oldValue])
+            if (oldValue !== value) backAttribs.set(key, oldValue);
           }
-
+          return backAttribs.toString();
-          return Changeset.makeAttribsString('=', backAttribs, apool);
         });
 
-        const oldAttribsAddition =
-            `*${Changeset.numToString(deletedAttrib)}*${Changeset.numToString(authorAttrib)}`;
-
         let textLeftToProcess = textBank;
 
         while (textLeftToProcess.length > 0) {
@@ -429,7 +418,7 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) {
             let textBankIndex = 0;
             consumeAttribRuns(lengthToProcess, (len, attribs, endsLine) => {
               // get the old attributes back
-              const oldAttribs = (undoBackToAttribs(attribs) || '') + oldAttribsAddition;
+              const oldAttribs = undoBackToAttribs(attribs);
 
               builder.insert(processText.substr(textBankIndex, len), oldAttribs);
               textBankIndex += len;

src/static/js/AttributeManager.js

@@ -1,7 +1,9 @@
 'use strict';
 
+const AttributeMap = require('./AttributeMap');
 const Changeset = require('./Changeset');
 const ChangesetUtils = require('./ChangesetUtils');
+const attributes = require('./attributes');
 const _ = require('./underscore');
 
 const lineMarkerAttribute = 'lmkr';
@@ -150,7 +152,7 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
     if (!aline) return '';
     const opIter = Changeset.opIterator(aline);
     if (!opIter.hasNext()) return '';
-    return Changeset.opAttributeValue(opIter.next(), attributeName, this.rep.apool) || '';
+    return AttributeMap.fromString(opIter.next().attribs, this.rep.apool).get(attributeName) || '';
   },
 
   /*
@@ -164,10 +166,7 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
     const opIter = Changeset.opIterator(aline);
     if (!opIter.hasNext()) return [];
     const op = opIter.next();
-    if (!op.attribs) return [];
+    return [...attributes.attribsFromString(op.attribs, this.rep.apool)];
-    const attributes = [];
-    Changeset.eachAttribNumber(op.attribs, (n) => attributes.push(this.rep.apool.getAttrib(n)));
-    return attributes;
   },
 
   /*
@@ -191,9 +190,7 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
       }
     }
 
-    const withIt = Changeset.makeAttribsString('+', [
+    const withIt = new AttributeMap(rep.apool).set(attributeName, 'true').toString();
-      [attributeName, 'true'],
-    ], rep.apool);
     const withItRegex = new RegExp(`${withIt.replace(/\*/g, '\\*')}(\\*|$)`);
     const hasIt = (attribs) => withItRegex.test(attribs);
 
@@ -274,10 +271,7 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
       currentOperation = opIter.next();
       currentPointer += currentOperation.chars;
       if (currentPointer <= column) continue;
-      const attributes = [];
+      return [...attributes.attribsFromString(currentOperation.attribs, this.rep.apool)];
-      Changeset.eachAttribNumber(
-          currentOperation.attribs, (n) => attributes.push(this.rep.apool.getAttrib(n)));
-      return attributes;
     }
     return [];
   },

src/static/js/Changeset.js

@@ -22,7 +22,9 @@
  * https://github.com/ether/pad/blob/master/infrastructure/ace/www/easysync2.js
  */
 
+const AttributeMap = require('./AttributeMap');
 const AttributePool = require('./AttributePool');
+const attributes = require('./attributes');
 const {padutils} = require('./pad_utils');
 
 /**
@@ -31,6 +33,15 @@ const {padutils} = require('./pad_utils');
  * @typedef {[string, string]} Attribute
  */
 
+/**
+ * A concatenated sequence of zero or more attribute identifiers, each one represented by an
+ * asterisk followed by a base-36 encoded attribute number.
+ *
+ * Examples: '', '*0', '*3*j*z*1q'
+ *
+ * @typedef {string} AttributeString
+ */
+
 /**
  * This method is called whenever there is an error in the sync process.
  *
@@ -236,15 +247,19 @@ const copyOp = (op1, op2 = exports.newOp()) => Object.assign(op2, op1);
  *
  * @param {('-'|'+'|'=')} opcode - The operator to use.
  * @param {string} text - The text to remove/add/keep.
- * @param {(string|Attribute[])} [attribs] - The attributes to apply to the operations. See
+ * @param {(Iterable<Attribute>|AttributeString)} [attribs] - The attributes to insert into the pool
- *     `makeAttribsString`.
+ *     (if necessary) and encode. If an attribute string, no checking is performed to ensure that
- * @param {?AttributePool} [pool] - See `makeAttribsString`.
+ *     the attributes exist in the pool, are in the canonical order, and contain no duplicate keys.
+ *     If this is an iterable of attributes, `pool` must be non-null.
+ * @param {?AttributePool} pool - Attribute pool. Required if `attribs` is an iterable of
+ *     attributes, ignored if `attribs` is an attribute string.
  * @yields {Op} One or two ops (depending on the presense of newlines) that cover the given text.
  * @returns {Generator<Op>}
  */
 const opsFromText = function* (opcode, text, attribs = '', pool = null) {
   const op = exports.newOp(opcode);
-  op.attribs = exports.makeAttribsString(opcode, attribs, pool);
+  op.attribs = typeof attribs === 'string'
+    ? attribs : new AttributeMap(pool).update(attribs || [], opcode === '+').toString();
   const lastNewlinePos = text.lastIndexOf('\n');
   if (lastNewlinePos < 0) {
     op.chars = text.length;
@@ -387,9 +402,9 @@ exports.smartOpAssembler = () => {
    * @deprecated Use `opsFromText` instead.
    * @param {('-'|'+'|'=')} opcode - The operator to use.
    * @param {string} text - The text to remove/add/keep.
-   * @param {(string|Attribute[])} attribs - The attributes to apply to the operations. See
+   * @param {(string|Iterable<Attribute>)} attribs - The attributes to apply to the operations.
-   *     `makeAttribsString`.
+   * @param {?AttributePool} pool - Attribute pool. Only required if `attribs` is an iterable of
-   * @param {?AttributePool} pool - See `makeAttribsString`.
+   *     attribute key, value pairs.
    */
   const appendOpWithText = (opcode, text, attribs, pool) => {
     padutils.warnWithStack('Changeset.smartOpAssembler().appendOpWithText() is deprecated; ' +
@@ -1109,20 +1124,11 @@ exports.mutateTextLines = (cs, lines) => {
   mut.close();
 };
 
-/**
- * Sorts an array of attributes by key.
- *
- * @param {Attribute[]} attribs - The array of attributes to sort in place.
- * @returns {Attribute[]} The `attribs` array.
- */
-const sortAttribs =
-    (attribs) => attribs.sort((a, b) => (a[0] > b[0] ? 1 : 0) - (a[0] < b[0] ? 1 : 0));
-
 /**
  * Composes two attribute strings (see below) into one.
  *
- * @param {string} att1 - first attribute string
+ * @param {AttributeString} att1 - first attribute string
- * @param {string} att2 - second attribue string
+ * @param {AttributeString} att2 - second attribue string
  * @param {boolean} resultIsMutation -
  * @param {AttributePool} pool - attribute pool
  * @returns {string}
@@ -1149,27 +1155,7 @@ exports.composeAttributes = (att1, att2, resultIsMutation, pool) => {
     return att2;
   }
   if (!att2) return att1;
-  const atts = new Map();
+  return AttributeMap.fromString(att1, pool).updateFromString(att2, !resultIsMutation).toString();
-  att1.replace(/\*([0-9a-z]+)/g, (_, a) => {
-    const [key, val] = pool.getAttrib(exports.parseNum(a));
-    atts.set(key, val);
-    return '';
-  });
-  att2.replace(/\*([0-9a-z]+)/g, (_, a) => {
-    const [key, val] = pool.getAttrib(exports.parseNum(a));
-    if (val || resultIsMutation) {
-      atts.set(key, val);
-    } else {
-      atts.delete(key);
-    }
-    return '';
-  });
-  const buf = exports.stringAssembler();
-  for (const att of sortAttribs([...atts])) {
-    buf.append('*');
-    buf.append(exports.numToString(pool.putAttrib(att)));
-  }
-  return buf.toString();
 };
 
 /**
@@ -1611,16 +1597,21 @@ exports.makeAttribution = (text) => {
  * Iterates over attributes in exports, attribution string, or attribs property of an op and runs
  * function func on them.
  *
+ * @deprecated Use `attributes.decodeAttribString()` instead.
  * @param {string} cs - changeset
  * @param {Function} func - function to call
  */
 exports.eachAttribNumber = (cs, func) => {
+  padutils.warnWithStack('Changeset.eachAttribNumber() is deprecated; ' +
+                         'use attributes.decodeAttribString() instead');
   let dollarPos = cs.indexOf('$');
   if (dollarPos < 0) {
     dollarPos = cs.length;
   }
   const upToDollar = cs.substring(0, dollarPos);
 
+  // WARNING: The following cannot be replaced with a call to `attributes.decodeAttribString()`
+  // because that function only works on attribute strings, not serialized operations or changesets.
   upToDollar.replace(/\*([0-9a-z]+)/g, (_, a) => {
     func(exports.parseNum(a));
     return '';
@@ -1784,33 +1775,44 @@ exports.isIdentity = (cs) => {
   return unpacked.ops === '' && unpacked.oldLen === unpacked.newLen;
 };
 
+/**
+ * @deprecated Use an AttributeMap instead.
+ */
+const attribsAttributeValue = (attribs, key, pool) => {
+  if (!attribs) return '';
+  for (const [k, v] of attributes.attribsFromString(attribs, pool)) {
+    if (k === key) return v;
+  }
+  return '';
+};
+
 /**
  * Returns all the values of attributes with a certain key in an Op attribs string.
  *
+ * @deprecated Use an AttributeMap instead.
  * @param {Op} op - Op
  * @param {string} key - string to search for
  * @param {AttributePool} pool - attribute pool
  * @returns {string}
  */
-exports.opAttributeValue = (op, key, pool) => exports.attribsAttributeValue(op.attribs, key, pool);
+exports.opAttributeValue = (op, key, pool) => {
+  padutils.warnWithStack('Changeset.opAttributeValue() is deprecated; use an AttributeMap instead');
+  return attribsAttributeValue(op.attribs, key, pool);
+};
 
 /**
  * Returns all the values of attributes with a certain key in an attribs string.
  *
- * @param {string} attribs - Attribute string
+ * @deprecated Use an AttributeMap instead.
+ * @param {AttributeString} attribs - Attribute string
  * @param {string} key - string to search for
  * @param {AttributePool} pool - attribute pool
  * @returns {string}
  */
 exports.attribsAttributeValue = (attribs, key, pool) => {
-  if (!attribs) return '';
+  padutils.warnWithStack('Changeset.attribsAttributeValue() is deprecated; ' +
-  let value = '';
+                         'use an AttributeMap instead');
-  exports.eachAttribNumber(attribs, (n) => {
+  return attribsAttributeValue(attribs, key, pool);
-    if (pool.getAttribKey(n) === key) {
-      value = pool.getAttribValue(n);
-    }
-  });
-  return value;
 };
 
 /**
@@ -1846,7 +1848,8 @@ exports.builder = (oldLen) => {
      */
     keep: (N, L, attribs, pool) => {
       o.opcode = '=';
-      o.attribs = (attribs && exports.makeAttribsString('=', attribs, pool)) || '';
+      o.attribs = typeof attribs === 'string'
+        ? attribs : new AttributeMap(pool).update(attribs || []).toString();
       o.chars = N;
       o.lines = (L || 0);
       assem.append(o);
@@ -1908,8 +1911,9 @@ exports.builder = (oldLen) => {
 /**
  * Constructs an attribute string from a sequence of attributes.
  *
+ * @deprecated Use `AttributeMap.prototype.toString()` or `attributes.attribsToString()` instead.
  * @param {string} opcode - The opcode for the Op that will get the resulting attribute string.
- * @param {?(Attribute[]|AttributeString)} attribs - The attributes to insert into the pool
+ * @param {?(Iterable<Attribute>|AttributeString)} attribs - The attributes to insert into the pool
  *     (if necessary) and encode. If an attribute string, no checking is performed to ensure that
  *     the attributes exist in the pool, are in the canonical order, and contain no duplicate keys.
  *     If this is an iterable of attributes, `pool` must be non-null.
@@ -1918,11 +1922,12 @@ exports.builder = (oldLen) => {
  * @returns {AttributeString}
  */
 exports.makeAttribsString = (opcode, attribs, pool) => {
+  padutils.warnWithStack(
+      'Changeset.makeAttribsString() is deprecated; ' +
+      'use AttributeMap.prototype.toString() or attributes.attribsToString() instead');
   if (!attribs || !['=', '+'].includes(opcode)) return '';
   if (typeof attribs === 'string') return attribs;
-  return sortAttribs(attribs.filter(([k, v]) => v || opcode === '='))
+  return new AttributeMap(pool).update(attribs, opcode === '+').toString();
-      .map((a) => `*${exports.numToString(pool.putAttrib(a))}`)
-      .join('');
 };
 
 /**
@@ -2085,17 +2090,15 @@ exports.inverse = (cs, lines, alines, pool) => {
     const csOp = csIter.next();
     if (csOp.opcode === '=') {
       if (csOp.attribs) {
-        const csAttribs = [];
+        const attribs = AttributeMap.fromString(csOp.attribs, pool);
-        exports.eachAttribNumber(csOp.attribs, (n) => csAttribs.push(pool.getAttrib(n)));
+        const undoBackToAttribs = cachedStrFunc((oldAttribsStr) => {
-        const undoBackToAttribs = cachedStrFunc((attribs) => {
+          const oldAttribs = AttributeMap.fromString(oldAttribsStr, pool);
-          const backAttribs = [];
+          const backAttribs = new AttributeMap(pool);
-          for (const [appliedKey, appliedValue] of csAttribs) {
+          for (const [key, value] of attribs) {
-            const oldValue = exports.attribsAttributeValue(attribs, appliedKey, pool);
+            const oldValue = oldAttribs.get(key) || '';
-            if (appliedValue !== oldValue) {
+            if (oldValue !== value) backAttribs.set(key, oldValue);
-              backAttribs.push([appliedKey, oldValue]);
-            }
           }
-          return exports.makeAttribsString('=', backAttribs, pool);
+          return backAttribs.toString();
         });
         consumeAttribRuns(csOp.chars, (len, attribs, endsLine) => {
           builder.keep(len, endsLine ? 1 : 0, undoBackToAttribs(attribs));

src/static/js/ace2_inner.js

@@ -18,6 +18,7 @@
  */
 let documentAttributeManager;
 
+const AttributeMap = require('./AttributeMap');
 const browser = require('./vendors/browser');
 const padutils = require('./pad_utils').padutils;
 const Ace2Common = require('./ace2_common');
@@ -1542,9 +1543,7 @@ function Ace2Inner(editorInfo, cssManagers) {
       }
     }
 
-    const withIt = Changeset.makeAttribsString('+', [
+    const withIt = new AttributeMap(rep.apool).set(attributeName, 'true').toString();
-      [attributeName, 'true'],
-    ], rep.apool);
     const withItRegex = new RegExp(`${withIt.replace(/\*/g, '\\*')}(\\*|$)`);
     const hasIt = (attribs) => withItRegex.test(attribs);
 
@@ -1608,9 +1607,7 @@ function Ace2Inner(editorInfo, cssManagers) {
     if (!(rep.selStart && rep.selEnd)) return;
 
     let selectionAllHasIt = true;
-    const withIt = Changeset.makeAttribsString('+', [
+    const withIt = new AttributeMap(rep.apool).set(attributeName, 'true').toString();
-      [attributeName, 'true'],
-    ], rep.apool);
     const withItRegex = new RegExp(`${withIt.replace(/\*/g, '\\*')}(\\*|$)`);
 
     const hasIt = (attribs) => withItRegex.test(attribs);
@@ -1820,22 +1817,15 @@ function Ace2Inner(editorInfo, cssManagers) {
         }
 
         let isNewTextMultiauthor = false;
-        const authorAtt = Changeset.makeAttribsString('+', (thisAuthor ? [
-          ['author', thisAuthor],
-        ] : []), rep.apool);
         const authorizer = cachedStrFunc((oldAtts) => {
-          if (isNewTextMultiauthor) {
+          const attribs = AttributeMap.fromString(oldAtts, rep.apool);
-            // prefer colors from DOM
+          if (!isNewTextMultiauthor || !attribs.has('author')) attribs.set('author', thisAuthor);
-            return Changeset.composeAttributes(authorAtt, oldAtts, true, rep.apool);
+          return attribs.toString();
-          } else {
-            // use this author's color
-            return Changeset.composeAttributes(oldAtts, authorAtt, true, rep.apool);
-          }
         });
 
         let foundDomAuthor = '';
         eachAttribRun(newAttribs, (start, end, attribs) => {
-          const a = Changeset.attribsAttributeValue(attribs, 'author', rep.apool);
+          const a = AttributeMap.fromString(attribs, rep.apool).get('author');
           if (a && a !== foundDomAuthor) {
             if (!foundDomAuthor) {
               foundDomAuthor = a;
@@ -2632,8 +2622,8 @@ function Ace2Inner(editorInfo, cssManagers) {
             const opIter = Changeset.opIterator(alineAttrs);
             while (opIter.hasNext()) {
               const op = opIter.next();
-              const authorId = Changeset.opAttributeValue(op, 'author', apool);
+              const authorId = AttributeMap.fromString(op.attribs, apool).get('author');
-              if (authorId !== '') authorIds.add(authorId);
+              if (authorId) authorIds.add(authorId);
             }
           }
           const idToName = new Map(parent.parent.pad.userList().map((a) => [a.userId, a.name]));

src/static/js/broadcast.js

@@ -26,6 +26,7 @@ const makeCSSManager = require('./cssmanager').makeCSSManager;
 const domline = require('./domline').domline;
 const AttribPool = require('./AttributePool');
 const Changeset = require('./Changeset');
+const attributes = require('./attributes');
 const linestylefilter = require('./linestylefilter').linestylefilter;
 const colorutils = require('./colorutils').colorutils;
 const _ = require('./underscore');
@@ -114,20 +115,18 @@ const loadBroadcastJS = (socket, sendSocketMsg, fireWhenAllScriptsAreLoaded, Bro
     },
 
     getActiveAuthors() {
-      const authors = [];
+      const authorIds = new Set();
-      const seenNums = {};
+      for (const aline of this.alines) {
-      const alines = this.alines;
+        const opIter = Changeset.opIterator(aline);
-      for (let i = 0; i < alines.length; i++) {
+        while (opIter.hasNext()) {
-        Changeset.eachAttribNumber(alines[i], (n) => {
+          const op = opIter.next();
-          if (seenNums[n]) return;
+          for (const [k, v] of attributes.attribsFromString(op.attribs, this.apool)) {
-          seenNums[n] = true;
+            if (k !== 'author') continue;
-          if (this.apool.getAttribKey(n) !== 'author') return;
+            if (v) authorIds.add(v);
-          const a = this.apool.getAttribValue(n);
+          }
-          if (a) authors.push(a);
+        }
-        });
       }
-      authors.sort();
+      return [...authorIds].sort();
-      return authors;
     },
   };
 

src/static/js/changesettracker.js

@@ -22,6 +22,7 @@
  * limitations under the License.
  */
 
+const AttributeMap = require('./AttributeMap');
 const AttributePool = require('./AttributePool');
 const Changeset = require('./Changeset');
 
@@ -141,7 +142,6 @@ const makeChangesetTracker = (scheduler, apool, aceCallbacksProvider) => {
 
         // Sanitize authorship: Replace all author attributes with this user's author ID in case the
         // text was copied from another author.
-        const authorAttr = Changeset.numToString(apool.putAttrib(['author', authorId]));
         const cs = Changeset.unpack(userChangeset);
         const iterator = Changeset.opIterator(cs.ops);
         let op;
@@ -150,18 +150,12 @@ const makeChangesetTracker = (scheduler, apool, aceCallbacksProvider) => {
         while (iterator.hasNext()) {
           op = iterator.next();
           if (op.opcode === '+') {
-            let newAttrs = '';
+            const attribs = AttributeMap.fromString(op.attribs, apool);
-
+            const oldAuthorId = attribs.get('author');
-            op.attribs.split('*').forEach((attrNum) => {
+            if (oldAuthorId != null && oldAuthorId !== authorId) {
-              if (!attrNum) return;
+              attribs.set('author', authorId);
-              const attr = apool.getAttrib(parseInt(attrNum, 36));
+              op.attribs = attribs.toString();
-              if (!attr) return;
+            }
-              if ('author' === attr[0]) {
-                // replace that author with the current one
-                newAttrs += `*${authorAttr}`;
-              } else { newAttrs += `*${attrNum}`; } // overtake all other attribs as is
-            });
-            op.attribs = newAttrs;
           }
           assem.append(op);
         }

src/static/js/contentcollector.js

@@ -26,6 +26,7 @@
 
 const _MAX_LIST_LEVEL = 16;
 
+const AttributeMap = require('./AttributeMap');
 const UNorm = require('unorm');
 const Changeset = require('./Changeset');
 const hooks = require('./pluginfw/hooks');
@@ -227,7 +228,7 @@ const makeContentCollector = (collectStyles, abrowser, apool, className2Author)
   };
 
   const _recalcAttribString = (state) => {
-    const lst = [];
+    const attribs = new AttributeMap(apool);
     for (const [a, count] of Object.entries(state.attribs)) {
       if (!count) continue;
       // The following splitting of the attribute name is a workaround
@@ -241,32 +242,31 @@ const makeContentCollector = (collectStyles, abrowser, apool, className2Author)
       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]]);
+        const [k, v] = attributeSplits;
+        if (v) attribs.set(k, v);
       } else {
         // the "normal" case, the attribute is just a switch
         // so set it true
-        lst.push([a, 'true']);
+        attribs.set(a, 'true');
       }
     }
     if (state.authorLevel > 0) {
-      const authorAttrib = ['author', state.author];
+      if (apool.putAttrib(['author', state.author], true) >= 0) {
-      if (apool.putAttrib(authorAttrib, true) >= 0) {
         // require that author already be in pool
         // (don't add authors from other documents, etc.)
-        lst.push(authorAttrib);
+        if (state.author) attribs.set('author', state.author);
       }
     }
-    state.attribString = Changeset.makeAttribsString('+', lst, apool);
+    state.attribString = attribs.toString();
   };
 
   const _produceLineAttributesMarker = (state) => {
     // TODO: This has to go to AttributeManager.
-    const attributes = [
+    const attribs = new AttributeMap(apool)
-      ['lmkr', '1'],
+        .set('lmkr', '1')
-      ['insertorder', 'first'],
+        .set('insertorder', 'first')
-      ...Object.entries(state.lineAttributes),
+        .update(Object.entries(state.lineAttributes).map(([k, v]) => [k, v || '']), true);
-    ];
+    lines.appendText('*', attribs.toString());
-    lines.appendText('*', Changeset.makeAttribsString('+', attributes, apool));
   };
   cc.startNewLine = (state) => {
     if (state) {

src/static/js/linestylefilter.js

@@ -31,6 +31,7 @@
 // requires: undefined
 
 const Changeset = require('./Changeset');
+const attributes = require('./attributes');
 const hooks = require('./pluginfw/hooks');
 const linestylefilter = {};
 const AttributeManager = require('./AttributeManager');
@@ -73,10 +74,8 @@ linestylefilter.getLineStyleFilter = (lineLength, aline, textAndClassFunc, apool
       let classes = '';
       let isLineAttribMarker = false;
 
-      // For each attribute number
+      for (const [key, value] of attributes.attribsFromString(attribs, apool)) {
-      Changeset.eachAttribNumber(attribs, (n) => {
+        if (!key || !value) continue;
-        const [key, value] = apool.getAttrib(n);
-        if (!key || !value) return;
         if (!isLineAttribMarker && AttributeManager.lineAttributes.indexOf(key) >= 0) {
           isLineAttribMarker = true;
         }
@@ -93,7 +92,7 @@ linestylefilter.getLineStyleFilter = (lineLength, aline, textAndClassFunc, apool
           const results = hooks.callAll('aceAttribsToClasses', {linestylefilter, key, value});
           classes += ` ${results.join(' ')}`;
         }
-      });
+      }
 
       if (isLineAttribMarker) classes += ` ${lineAttributeMarker}`;
       return classes.substring(1);

src/tests/backend/specs/contentcollector.js

@@ -12,6 +12,7 @@
 const AttributePool = require('../../../static/js/AttributePool');
 const Changeset = require('../../../static/js/Changeset');
 const assert = require('assert').strict;
+const attributes = require('../../../static/js/attributes');
 const contentcollector = require('../../../static/js/contentcollector');
 const jsdom = require('jsdom');
 
@@ -348,7 +349,9 @@ describe(__filename, function () {
           const opIter = Changeset.opIterator(aline);
           while (opIter.hasNext()) {
             const op = opIter.next();
-            Changeset.eachAttribNumber(op.attribs, (n) => assert(n < knownAttribs.length));
+            for (const n of attributes.decodeAttribString(op.attribs)) {
+              assert(n < knownAttribs.length);
+            }
           }
         }
         const cc = contentcollector.makeContentCollector(true, null, apool);
@@ -375,16 +378,9 @@ describe(__filename, function () {
           const opIter = Changeset.opIterator(aline);
           while (opIter.hasNext()) {
             const op = opIter.next();
-            const gotOpAttribs = [];
+            const gotOpAttribs = [...attributes.attribsFromString(op.attribs, apool)];
             gotAlineAttribs.push(gotOpAttribs);
-            const wantOpAttribs = [];
+            wantAlineAttribs.push(attributes.sort([...gotOpAttribs]));
-            wantAlineAttribs.push(wantOpAttribs);
-            Changeset.eachAttribNumber(op.attribs, (n) => {
-              const attrib = apool.getAttrib(n);
-              gotOpAttribs.push(attrib);
-              wantOpAttribs.push(attrib);
-            });
-            wantOpAttribs.sort(([keyA], [keyB]) => (keyA > keyB ? 1 : 0) - (keyA < keyB ? 1 : 0));
           }
         }
         assert.deepEqual(gotAttribs, wantAttribs);