Treora commented on issue #120:
URL: 
https://github.com/apache/incubator-annotator/issues/120#issuecomment-1083611111


   In which situation are empty text nodes added? That sounds like a potential 
bug. Only the start and/or end node should be splitted, and only if it is 
partially selected by the range. See [these 
lines](https://github.com/apache/incubator-annotator/blob/0821e0640a811d87f111023ee406469c0dd47deb/packages/dom/src/highlight-text.ts#L75-L90):
   
     // If the start or end node is a text node and only partly in the range, 
split it.
     if (isTextNode(range.startContainer) && range.startOffset > 0) {
       const endOffset = range.endOffset; // (this may get lost when the 
splitting the node)
       const createdNode = range.startContainer.splitText(range.startOffset);
       if (range.endContainer === range.startContainer) {
         // If the end was in the same container, it will now be in the newly 
created node.
         range.setEnd(createdNode, endOffset - range.startOffset);
       }
       range.setStart(createdNode, 0);
     }
     if (
       isTextNode(range.endContainer) &&
       range.endOffset < range.endContainer.length
     ) {
       range.endContainer.splitText(range.endOffset);
     }
   
   As discussed in issue #80, the highlighter does not run `.normalize()`, 
simply to not touch dom nodes when it’s not necessary. However, as written 
there, I’d be happy to add a few lines that undo the splitting of the start and 
end nodes, if any splitting happened, so that removing the highlight becomes a 
“perfect undo”.
   
   But again, empty text nodes should not appear due to our highlighter, so I’d 
be glad if you have an example case to help debugging this!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@annotator.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to