Treora commented on issue #47: URL: https://github.com/apache/incubator-annotator/issues/47#issuecomment-663510334
Since recently we have a simple highlighter, which wraps text nodes in `<mark>` elements, and ignores any existing `<mark>`s to allow for nested use. I just created some simple tests (see PR #84), including ones inspired by the examples above, that deal with situations like this: `'<b>lorem ipsum <mark>dolor <mark2>am</mark2></mark><mark2>et yada</mark2> yada</b>'`. Note there is a difficulty with how a Range behaves when the DOM is modified: running highlight with our current highlighter can mess up other Range objects that point at the same text nodes. So this can cause trouble: ``` range1 = anchor(annotation1); range2 = anchor(annotation2); highlight(range1); // this may mess up range2 highlight(range2); // highlights some unintended target. ``` A solution is to anchor&highlight as a single action: considering the Range a ephemeral pointer: ``` range1 = anchor(annotation1); highlight(range1); range2 = anchor(annotation2); highlight(range2); ``` Our generator-based approach to anchoring should help to do this right, but still it is a pitfall that I’m not very happy with. Some ideas for avoiding this problem: - Using a highlighter that does not modify the document content; some highlighter approaches add an (svg) element to the end of the `<body>` and display it on the right spot using absolute positioning. While solving this issue, it does create others (e.g. need to reposition the element when text reflows). - Stop using Range for our ‘hydrated’ selector, i.e. as our way to point at a part of the DOM. We could e.g. try implement a ‘RobustRange’ that updates its start-&endContainer&-Offset as needed in one way or another. For the time being, or if we decide not to fix this, we should probably warn users in any documentation and examples that Ranges are perishable. Note that the behaviour of Range actually differs between the current jsdom and web browsers, so it is important to run relevant tests in a browser (use our `yarn start` command) to ensure the tests pass there too. On the bright side, I added [tests](https://github.com/apache/incubator-annotator/blob/d4d933c61bb559221b5dd896fde399fc53adedba/packages/dom/test/highlight-range/highlight-range.test.ts#L172-L198) to check if one can remove highlights in arbitrary order, and that seems to work as intended. This should give the freedom to ignore the tree structure and treat highlights as being independent from each other. Please suggest/write other scenarios that we should include in our tests. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
