More EPUB CFI fixes

src/calibre/ebooks/oeb/display/cfi.coffee

@@ -8,8 +8,7 @@
  (http://code.google.com/p/epub-revision/source/browse/trunk/src/samples/cfi/epubcfi.js)
  Improvements with respect to that code:
  1. Works on all browsers (WebKit, Firefox and IE >= 8)
- 2. Works if the point is after the last text character in an element
- 3. Works for elements that are scrollable (i.e. have their own scrollbars)
+ 2. Works for elements that are scrollable (i.e. have their own scrollbars)
 
  To check if this script is compatible with the current browser, call
  window.cfi.is_compatible() it will throw an exception if not compatible.
@@ -127,9 +126,9 @@ find_offset_for_point = (x, y, node, cdoc) ->
 
     if not last_child
         throw "#{node} has no children"
-    # The point must be after the last bit of text
-    pos = 0
-    return [last_child, last_child.nodeValue.length]
+    # The point must be after the last bit of text/in the padding/border, we dont know
+    # how to get a good point in this case
+    throw "Point (#{x}, #{y}) is in the padding/border of #{node}, so cannot calculate offset"
 
 # }}}
 
@@ -192,9 +191,11 @@ class CanonicalFragmentIdentifier
                 offset or= 0
                 while true
                     p = node.previousSibling
-                    if (p?.nodeType not in [3, 4, 5, 6])
+                    if not p or p.nodeType > 8
                         break
-                    offset += p.nodeValue.length
+                    # log("previous sibling:"+ p + " " + p?.nodeType + " length: " + p?.nodeValue?.length)
+                    if p.nodeType not in [2, 8] and p.nodeValue?.length?
+                        offset += p.nodeValue.length
                     node = p
                 cfi = ":" + offset + cfi
             else # Not handled
@@ -332,6 +333,7 @@ class CanonicalFragmentIdentifier
                     if nn.nodeType in [3, 4, 5, 6] and nn.nodeValue?.length # Text node, entity, cdata
                         next = nn
                         break
+                    node = nn
                 if not next
                     if offset > len
                         error = "Offset out of range: #{ offset }"
@@ -391,14 +393,11 @@ class CanonicalFragmentIdentifier
             py = ((y + cwin.scrollY - target.offsetTop)*100)/target.offsetHeight
             tail = "#{ tail }@#{ fstr px },#{ fstr py }"
         else if name != 'audio'
-            if cdoc.caretRangeFromPoint # WebKit
-                range = cdoc.caretRangeFromPoint(x, y)
-                if range
-                    target = range.startContainer
-                    offset = range.startOffset
-                else
-                    throw "Failed to find range from point (#{ x }, #{ y })"
-            else if cdoc.createRange
+            # Get the test offset
+            # We use a custom function instead of caretRangeFromPoint as
+            # caretRangeFromPoint does weird things when the point falls in the
+            # padding of the element
+            if cdoc.createRange
                 [target, offset] = find_offset_for_point(x, y, target, cdoc)
             else
                 throw this.CREATE_RANGE_ERR

src/calibre/ebooks/oeb/display/test-cfi/cfi-test.coffee

@@ -28,10 +28,15 @@ mark_and_reload = (evt) ->
     # Remove image in case the click was on the image itself, we want the cfi to
     # be on the underlying element
     ms = document.getElementById("marker")
-    ms.parentNode?.removeChild(ms)
+    if ms
+        ms.parentNode?.removeChild(ms)
 
     fn = () ->
-        window.current_cfi = window.cfi.at(evt.clientX, evt.clientY)
+        try
+            window.current_cfi = window.cfi.at(evt.clientX, evt.clientY)
+        catch err
+            log("Failed to calculate cfi: #{ err }")
+            return
         if window.current_cfi
             epubcfi = "#epubcfi(#{ window.current_cfi })"
             newloc = window.location.href.replace(/#.*$/, '') + epubcfi

src/calibre/ebooks/oeb/display/test-cfi/index.html

@@ -33,15 +33,21 @@
                 border: solid 1px black;
                 padding: 2em;
             }
+            #whitespace {
+                border: 20px solid gray;
+                margin: 20px;
+                padding: 20px;
+            }
         </style>
     </head>
     <body>
         <div id="container">
             <h1 id="first-h1">Testing EPUB CFI</h1>
             <div id="current-cfi">Current CFI:&nbsp;None</div>
+            <p><a href="/">Reset</a></p>
             <h2>A div with scrollbars</h2>
             <p>Scroll down and click on some elements. Make sure to hit both bold and not bold text</p>
-            <div id="overflow"> But I must explain to you how all this mistaken
+            <div id="overflow">But I must explain to you how all this mistaken
                 idea of denouncing pleasure and praising pain was born and I
                 will give you a complete account of the system, and expound the
                 actual teachings of the great explorer of the truth, the
@@ -62,9 +68,34 @@
                 demoralized by the charms of pleasure of the moment, so blinded
                 by desire, that they cannot foresee
             </div>
-            <h2>Some entities</h2>
-            <p>Entities: &amp; &copy; &sect; &gt;</p>
-            <p>An invisible CDATA: <![CDATA[CDATA]]> followed by some text</p>
+            <h2>Some entities and comments</h2>
+            <p>Entities: &amp; &copy; &sect; &gt; some text after entities</p>
+            <p>An invisible Comment: <!-- aaaaaa --> followed by some text</p>
+            <p>An invalid (in HTML) CDATA: <![CDATA[CDATA]]> followed by some text</p>
+            <h2>Margins padding borders</h2>
+            <p>Try clicking in the margins, borders and padding</p>
+
+            <p id="whitespace">But I must explain to you how all this mistaken
+                idea of denouncing pleasure and praising pain was born and I
+                will give you a complete account of the system, and expound the
+                actual teachings of the great explorer of the truth, the
+                master-builder of human happiness. No one rejects, dislikes, or
+                avoids pleasure itself, because it is pleasure, but because
+                those who do not know how to pursue pleasure rationally
+                encounter consequences that are extremely painful. Nor again is
+                there anyone who <b>loves</b> or pursues or desires to obtain pain of
+                itself, because it is pain, but because occasionally
+                circumstances occur in which toil and pain can procure him some
+                great pleasure. To take a trivial example, which of us ever
+                undertakes laborious physical exercise, except to obtain some
+                advantage from it? But who has any right to find fault with a
+                man who chooses to enjoy a pleasure that has no annoying
+                consequences, or one who avoids a pain that produces no
+                resultant pleasure? On the other hand, we denounce with
+                righteous indignation and dislike men who are so beguiled and
+                demoralized by the charms of pleasure of the moment, so blinded
+                by desire, that they cannot foresee</p>
+
         </div>
         <img id="marker" style="position: absolute; display:none; z-index:10" src="marker.png" alt="Marker" />
     </body>