From f1eb7a25a6490941021a3c777e52fc00c4cfd480 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Fri, 19 Nov 2021 00:51:25 -0500 Subject: [PATCH] Changeset: Migrate to the new attribute API --- CHANGELOG.md | 12 ++ doc/api/hooks_server-side.md | 3 +- src/node/handler/PadMessageHandler.js | 38 ++---- src/node/utils/ExportHelper.js | 6 +- src/node/utils/ExportHtml.js | 5 +- src/node/utils/ExportTxt.js | 5 +- src/node/utils/padDiff.js | 43 +++---- src/static/js/AttributeManager.js | 18 +-- src/static/js/Changeset.js | 131 ++++++++++---------- src/static/js/ace2_inner.js | 28 ++--- src/static/js/broadcast.js | 25 ++-- src/static/js/changesettracker.js | 20 ++- src/static/js/contentcollector.js | 26 ++-- src/static/js/linestylefilter.js | 9 +- src/tests/backend/specs/contentcollector.js | 16 +-- 15 files changed, 175 insertions(+), 210 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8850e4036..799b875e3 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/doc/api/hooks_server-side.md b/doc/api/hooks_server-side.md index 476c3d050..38cca9baa 100644 --- a/doc/api/hooks_server-side.md +++ b/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}`; }; diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js index 0735ce97f..868ca5b4a 100644 --- a/src/node/handler/PadMessageHandler.js +++ b/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) => { - if (!attr) return; - - attr = wireApool.getAttrib(Changeset.parseNum(attr)); - if (!attr) return; - - // 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(`Author ${thisSession.author} tried to submit changes as author ` + - `${attr[1]} in changeset ${changeset}`); - } - }); + // Besides verifying the author attribute, this serves a second purpose: + // AttributeMap.fromString() ensures that all attribute numbers are valid (it will throw if + // an attribute number isn't in the pool). + const opAuthorId = AttributeMap.fromString(op.attribs, wireApool).get('author'); + if (opAuthorId && opAuthorId !== thisSession.author) { + throw new Error(`Author ${thisSession.author} tried to submit changes as author ` + + `${opAuthorId} in changeset ${changeset}`); + } } // ex. adoptChangesetAttribs @@ -758,11 +745,8 @@ const _correctMarkersInPad = (atext, apool) => { let offset = 0; while (iter.hasNext()) { const op = iter.next(); - - const hasMarker = _.find( - AttributeManager.lineAttributes, - (attribute) => Changeset.opAttributeValue(op, attribute, apool)) !== undefined; - + const attribs = AttributeMap.fromString(op.attribs, apool); + const hasMarker = AttributeManager.lineAttributes.some((a) => attribs.has(a)); if (hasMarker) { for (let i = 0; i < op.chars; i++) { if (offset > 0 && text.charAt(offset - 1) !== '\n') { diff --git a/src/node/utils/ExportHelper.js b/src/node/utils/ExportHelper.js index ba71269d1..401fad70b 100644 --- a/src/node/utils/ExportHelper.js +++ b/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; } diff --git a/src/node/utils/ExportHtml.js b/src/node/utils/ExportHtml.js index bc50da77b..800798f9c 100644 --- a/src/node/utils/ExportHtml.js +++ b/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--) { diff --git a/src/node/utils/ExportTxt.js b/src/node/utils/ExportTxt.js index 0ff7ded83..1d7ce5469 100644 --- a/src/node/utils/ExportTxt.js +++ b/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) { diff --git a/src/node/utils/padDiff.js b/src/node/utils/padDiff.js index 02fbdd99f..a8841cf22 100644 --- a/src/node/utils/padDiff.js +++ b/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 = []; - Changeset.eachAttribNumber(changeset, (attrNum) => { - attributes.push(attrNum); - }); + const [appliedAttribute, anotherAttribute] = + attributes.attribsFromString(clearOperator.attribs, this._pad.pool); - // check that this changeset uses only one attribute - if (attributes.length !== 1) { - return false; - } - - const appliedAttribute = this._pad.pool.getAttrib(attributes[0]); + // Check that the operation has exactly one attribute. + if (appliedAttribute == null || anotherAttribute != null) return false; // 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]); - 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 attribs = AttributeMap.fromString(csOp.attribs, apool); 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); - if (oldValue !== value) backAttribs.push([key, oldValue]) + const oldValue = oldAttribs.get(key); + if (oldValue !== value) backAttribs.set(key, oldValue); } - - return Changeset.makeAttribsString('=', backAttribs, apool); + return backAttribs.toString(); }); - 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; diff --git a/src/static/js/AttributeManager.js b/src/static/js/AttributeManager.js index 760422427..ebef74c07 100644 --- a/src/static/js/AttributeManager.js +++ b/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 []; - const attributes = []; - Changeset.eachAttribNumber(op.attribs, (n) => attributes.push(this.rep.apool.getAttrib(n))); - return attributes; + return [...attributes.attribsFromString(op.attribs, this.rep.apool)]; }, /* @@ -191,9 +190,7 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({ } } - const withIt = Changeset.makeAttribsString('+', [ - [attributeName, 'true'], - ], rep.apool); + const withIt = new AttributeMap(rep.apool).set(attributeName, 'true').toString(); 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 = []; - Changeset.eachAttribNumber( - currentOperation.attribs, (n) => attributes.push(this.rep.apool.getAttrib(n))); - return attributes; + return [...attributes.attribsFromString(currentOperation.attribs, this.rep.apool)]; } return []; }, diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js index 71d7fe115..11494a5f7 100644 --- a/src/static/js/Changeset.js +++ b/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 - * `makeAttribsString`. - * @param {?AttributePool} [pool] - See `makeAttribsString`. + * @param {(Iterable|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. + * @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} */ 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 - * `makeAttribsString`. - * @param {?AttributePool} pool - See `makeAttribsString`. + * @param {(string|Iterable)} attribs - The attributes to apply to the operations. + * @param {?AttributePool} pool - Attribute pool. Only required if `attribs` is an iterable of + * 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 {string} att2 - second attribue string + * @param {AttributeString} att1 - first attribute 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(); - 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(); + return AttributeMap.fromString(att1, pool).updateFromString(att2, !resultIsMutation).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 ''; - let value = ''; - exports.eachAttribNumber(attribs, (n) => { - if (pool.getAttribKey(n) === key) { - value = pool.getAttribValue(n); - } - }); - return value; + padutils.warnWithStack('Changeset.attribsAttributeValue() is deprecated; ' + + 'use an AttributeMap instead'); + return attribsAttributeValue(attribs, key, pool); }; /** @@ -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|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 === '=')) - .map((a) => `*${exports.numToString(pool.putAttrib(a))}`) - .join(''); + return new AttributeMap(pool).update(attribs, opcode === '+').toString(); }; /** @@ -2085,17 +2090,15 @@ exports.inverse = (cs, lines, alines, pool) => { const csOp = csIter.next(); if (csOp.opcode === '=') { if (csOp.attribs) { - const csAttribs = []; - exports.eachAttribNumber(csOp.attribs, (n) => csAttribs.push(pool.getAttrib(n))); - const undoBackToAttribs = cachedStrFunc((attribs) => { - const backAttribs = []; - for (const [appliedKey, appliedValue] of csAttribs) { - const oldValue = exports.attribsAttributeValue(attribs, appliedKey, pool); - if (appliedValue !== oldValue) { - backAttribs.push([appliedKey, oldValue]); - } + const attribs = AttributeMap.fromString(csOp.attribs, pool); + const undoBackToAttribs = cachedStrFunc((oldAttribsStr) => { + const oldAttribs = AttributeMap.fromString(oldAttribsStr, pool); + const backAttribs = new AttributeMap(pool); + for (const [key, value] of attribs) { + const oldValue = oldAttribs.get(key) || ''; + if (oldValue !== value) backAttribs.set(key, oldValue); } - return exports.makeAttribsString('=', backAttribs, pool); + return backAttribs.toString(); }); consumeAttribRuns(csOp.chars, (len, attribs, endsLine) => { builder.keep(len, endsLine ? 1 : 0, undoBackToAttribs(attribs)); diff --git a/src/static/js/ace2_inner.js b/src/static/js/ace2_inner.js index 675427019..46c6ca579 100644 --- a/src/static/js/ace2_inner.js +++ b/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('+', [ - [attributeName, 'true'], - ], rep.apool); + const withIt = new AttributeMap(rep.apool).set(attributeName, 'true').toString(); 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('+', [ - [attributeName, 'true'], - ], rep.apool); + const withIt = new AttributeMap(rep.apool).set(attributeName, 'true').toString(); 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) { - // prefer colors from DOM - return Changeset.composeAttributes(authorAtt, oldAtts, true, rep.apool); - } else { - // use this author's color - return Changeset.composeAttributes(oldAtts, authorAtt, true, rep.apool); - } + const attribs = AttributeMap.fromString(oldAtts, rep.apool); + if (!isNewTextMultiauthor || !attribs.has('author')) attribs.set('author', thisAuthor); + return attribs.toString(); }); 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); - if (authorId !== '') authorIds.add(authorId); + const authorId = AttributeMap.fromString(op.attribs, apool).get('author'); + if (authorId) authorIds.add(authorId); } } const idToName = new Map(parent.parent.pad.userList().map((a) => [a.userId, a.name])); diff --git a/src/static/js/broadcast.js b/src/static/js/broadcast.js index c778f6909..5b19acf88 100644 --- a/src/static/js/broadcast.js +++ b/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 seenNums = {}; - const alines = this.alines; - for (let i = 0; i < alines.length; i++) { - Changeset.eachAttribNumber(alines[i], (n) => { - if (seenNums[n]) return; - seenNums[n] = true; - if (this.apool.getAttribKey(n) !== 'author') return; - const a = this.apool.getAttribValue(n); - if (a) authors.push(a); - }); + const authorIds = new Set(); + for (const aline of this.alines) { + const opIter = Changeset.opIterator(aline); + while (opIter.hasNext()) { + const op = opIter.next(); + for (const [k, v] of attributes.attribsFromString(op.attribs, this.apool)) { + if (k !== 'author') continue; + if (v) authorIds.add(v); + } + } } - authors.sort(); - return authors; + return [...authorIds].sort(); }, }; diff --git a/src/static/js/changesettracker.js b/src/static/js/changesettracker.js index cfb6c88a3..c45a253d4 100644 --- a/src/static/js/changesettracker.js +++ b/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 = ''; - - op.attribs.split('*').forEach((attrNum) => { - if (!attrNum) return; - const attr = apool.getAttrib(parseInt(attrNum, 36)); - 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; + const attribs = AttributeMap.fromString(op.attribs, apool); + const oldAuthorId = attribs.get('author'); + if (oldAuthorId != null && oldAuthorId !== authorId) { + attribs.set('author', authorId); + op.attribs = attribs.toString(); + } } assem.append(op); } diff --git a/src/static/js/contentcollector.js b/src/static/js/contentcollector.js index 54c628804..59e726232 100644 --- a/src/static/js/contentcollector.js +++ b/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(authorAttrib, true) >= 0) { + if (apool.putAttrib(['author', state.author], 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 = [ - ['lmkr', '1'], - ['insertorder', 'first'], - ...Object.entries(state.lineAttributes), - ]; - lines.appendText('*', Changeset.makeAttribsString('+', attributes, apool)); + const attribs = new AttributeMap(apool) + .set('lmkr', '1') + .set('insertorder', 'first') + .update(Object.entries(state.lineAttributes).map(([k, v]) => [k, v || '']), true); + lines.appendText('*', attribs.toString()); }; cc.startNewLine = (state) => { if (state) { diff --git a/src/static/js/linestylefilter.js b/src/static/js/linestylefilter.js index 0821889c7..f6bcbb54e 100644 --- a/src/static/js/linestylefilter.js +++ b/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 - Changeset.eachAttribNumber(attribs, (n) => { - const [key, value] = apool.getAttrib(n); - if (!key || !value) return; + for (const [key, value] of attributes.attribsFromString(attribs, apool)) { + if (!key || !value) continue; 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); diff --git a/src/tests/backend/specs/contentcollector.js b/src/tests/backend/specs/contentcollector.js index a6ff8c2d7..c2cee5338 100644 --- a/src/tests/backend/specs/contentcollector.js +++ b/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(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)); + wantAlineAttribs.push(attributes.sort([...gotOpAttribs])); } } assert.deepEqual(gotAttribs, wantAttribs);