Clean up implementation of mark attributes

Issue #5673

Author: marijnh

doc/manual.html

@@ -1716,9 +1716,10 @@ <h3 id="api_marker">Text-marking methods</h3>
         to <code>startStyle</code>, but for the rightmost span.</dd>
         <dt id="mark_css"><code><strong>css</strong>: string</code></dt>
         <dd>A string of CSS to be applied to the covered text. For example <code>"color: #fe3"</code>.</dd>
-        <dt id="mark_attributes"><code><strong>attributes</strong>:
-        object</code></dt><dd>When given, will give the nodes created
-        for this span a HTML <code>attributes</code> with the given value.</dd>
+        <dt id="mark_attributes"><code><strong>attributes</strong>: object</code></dt>
+        <dd>When given, add the attributes in the given object to the
+        elements created for the marked text. Adding <code>class</code> or
+        <code>style</code> attributes this way is not supported.</dd>
         <dt id="mark_shared"><code><strong>shared</strong>: boolean</code></dt><dd>When the
         target document is <a href="#linkedDoc">linked</a> to other
         documents, you can set <code>shared</code> to true to make the

src/line/line_data.js

@@ -184,11 +184,8 @@ function buildToken(builder, text, style, startStyle, endStyle, css, attributes)
     if (endStyle) fullStyle += endStyle
     let token = elt("span", [content], fullStyle, css)
     if (attributes) {
-      for (let attr in attributes){
-        if (attributes.hasOwnProperty(attr) && attr !== "style" && attr !== "class") {
-          token.setAttribute(attr, attributes[attr])
-        }
-      }
+      for (let attr in attributes) if (attributes.hasOwnProperty(attr) && attr != "style" && attr != "class")
+        token.setAttribute(attr, attributes[attr])
     }
     return builder.content.appendChild(token)
   }
@@ -263,7 +260,7 @@ function insertLineContent(line, builder, styles) {
   for (;;) {
     if (nextChange == pos) { // Update current marker set
       spanStyle = spanEndStyle = spanStartStyle = css = ""
-      attributes = {}
+      attributes = null
       collapsed = null; nextChange = Infinity
       let foundBookmarks = [], endStyles
       for (let j = 0; j < spans.length; ++j) {
@@ -281,8 +278,11 @@ function insertLineContent(line, builder, styles) {
           if (m.endStyle && sp.to == nextChange) (endStyles || (endStyles = [])).push(m.endStyle, sp.to)
           // support for the old title property
           // https://github.com/codemirror/CodeMirror/pull/5673
-          if (m.title) {attributes.title = m.title}
-          if (m.attributes) { attributes = m.attributes }
+          if (m.title) (attributes || (attributes = {})).title = m.title
+          if (m.attributes) {
+            for (let attr in m.attributes)
+              (attributes || (attributes = {}))[attr] = m.attributes[attr]
+          }
           if (m.collapsed && (!collapsed || compareCollapsedMarkers(collapsed.marker, m) < 0))
             collapsed = sp
         } else if (sp.from > pos && nextChange > sp.from) {

src/model/mark_text.js

@@ -211,7 +211,8 @@ export function markText(doc, from, to, options, type) {
     if (updateMaxLine) cm.curOp.updateMaxLine = true
     if (marker.collapsed)
       regChange(cm, from.line, to.line + 1)
-    else if (marker.className || marker.startStyle || marker.endStyle || marker.css || marker.attributes )
+    else if (marker.className || marker.startStyle || marker.endStyle || marker.css ||
+             marker.attributes || marker.title)
       for (let i = from.line; i <= to.line; i++) regLineChange(cm, i, "text")
     if (marker.atomic) reCheckSelection(cm.doc)
     signalLater(cm, "markerAdded", cm, marker)

test/test.js

@@ -549,9 +549,10 @@ testCM("markTextCSS", function(cm) {
   function present() {
     var spans = cm.display.lineDiv.getElementsByTagName("span");
     for (var i = 0; i < spans.length; i++)
-      if (spans[i].style.color == "cyan" && spans[i].textContent == "cdefg") return true;
+      if (spans[i].style.color == "cyan" && spans[i].textContent == "cdef") return true;
   }
   var m = cm.markText(Pos(0, 2), Pos(0, 6), {css: "color: cyan"});
+  is(present());
   m.clear();
   is(!present());
 }, {value: "abcdefgh"});
@@ -560,9 +561,10 @@ testCM("markTextWithAttributes", function(cm) {
   function present() {
     var spans = cm.display.lineDiv.getElementsByTagName("span");
     for (var i = 0; i < spans.length; i++)
-      if (spans[i].getAttribute('label') == "label" && span[i].textContent == "cdefg") return true;
+      if (spans[i].getAttribute("label") == "label" && spans[i].textContent == "cdef") return true;
   }
   var m = cm.markText(Pos(0, 2), Pos(0, 6), {attributes: {label: "label"}});
+  is(present());
   m.clear();
   is(!present());
 }, {value: "abcdefgh"});