Unprecise CFI for getFirstVisibleElementCfi() / cfi_navigation_logic.js

[edas]

The CFI returned by getFirstVisibleElementCfi() in cfi_navigation_logic.js is unprecise.

Procedure to reproduce

Open the redium-js-viewer with the embedded book 'accessible epub 3'. Go to chapter 2 and turn a few pages. Launch getFirstVisibleElementCfi()

Actual result

The partial CFI returned is always ""/4/2[building_a_better_epub]/2@0:xx". This targets the first <section> of the chapter, which include the whole text content of the chapter, then add a positionning in % in that chapter.

Expected result

CFI returned should be more precise and return an CFI pointing to an inner <p>inside the <section>, like "/4/2[building_a_better_epub]/10/32/12@0:xx".

Explaination

This is because the first child of the <section> element, which is a blank text node, is reported by both Chrome and Firefox as visible on all columns in a multi-columns layout. When walking the DOM to find the first visible text node, this one is always checked first and always answer "yes".

The code should ignore blank text nodes but this one is not ignored. Probably because it contains tabulations, carriage returns or other characters than space and line feed.

Fix

In cfi_navigation_logic.js :

function isValidTextNode(node) {
    if(node.nodeType === Node.TEXT_NODE) {
        return (! node.nodeValue.match(/^\s*$/) );
     }
     return false;
}

This replace the all code that just strip spaces and line feeds then check that the length is not 0.

Problem

I did not send a PR because when fixing this bug, I trigger a browser bug on Firefox which forbids Firefox from being able to jump to these more precise CFI (the getClientRects() of the targeted element are always totally incoherent).

[edas]

See https://github.com/edas/readium-shared-js/commit/778a4358c02c0f4bb4dd24960027249c9a85399d

[dmitrym0]

Hi @edas, I believe we may have a fix for this. @jccr can you comment?

[danielweck]

@edas, @dmitrym0, @jccr, can you please continue working on this issue in the correct repository (readium-shared-js)

readium/readium-shared-js#35

Closing this one.