readium / readium-shared-js

Repository for the shared JavaScript libraries that are used in the SDK-Launchers and other applications developed on top of the SDK
BSD 3-Clause "New" or "Revised" License
78 stars 102 forks source link

Impossible to highlight two portions of text starting at the same offset #147

Open ClementHard opened 9 years ago

ClementHard commented 9 years ago

I have an annotation system and a search engine using the annotation module to highlight contents. So I want to be able to highlight some portion of text twice.

My problem is that if I highlight two CFI with the same start offset, i got an error.

For example : Annotated text : /4/2/8,/1:4,/1:30 Search result : /4/2/8,/1:4,/1:17

On the second call i got the following error : "Selection start and end must be text nodes" annotations_module.js (619)

The annotation module should ignore the cfi marker node.

danielweck commented 9 years ago

It sounds like the CFI "blacklist" is not taken into account when making a selection, if so, definitely a bug.

ClementHard commented 9 years ago

Adding the following code here resolve my problem :

        while (startNode.nextSibling && startNode.classList && (startNode.classList.contains("cfi-marker") || startNode.classList.contains("mo-cfi-highlight"))) {
            startNode = startNode.nextSibling;
        }

        while (endNode.previousSibling && endNode.classList && (endNode.classList.contains("cfi-marker") || endNode.classList.contains("mo-cfi-highlight"))) {
            endNode = endNode.previousSibling;
        }
danielweck commented 9 years ago

@JCCR would it make sense to migrate this "selection-to-cfi" code from annotation_module to the CFI library, as a common utility? (whatever helps harmonizing the blacklisting API)

danielweck commented 9 years ago

Follow-up to my above comment: https://github.com/readium/readium-shared-js/issues/201