diff --git a/chrome/content/zotero/browser.js b/chrome/content/zotero/browser.js index 9e450714b..ef4ff406a 100644 --- a/chrome/content/zotero/browser.js +++ b/chrome/content/zotero/browser.js @@ -306,6 +306,7 @@ var Zotero_Browser = new function() { // clear data object var tab = _getTabObject(browser); + if(!tab) return; // save annotations if(tab.page && tab.page.annotations) tab.page.annotations.save(); diff --git a/chrome/content/zotero/xpcom/annotate.js b/chrome/content/zotero/xpcom/annotate.js index b4f803a58..74cd6f96a 100644 --- a/chrome/content/zotero/xpcom/annotate.js +++ b/chrome/content/zotero/xpcom/annotate.js @@ -20,6 +20,8 @@ ***** END LICENSE BLOCK ***** */ +const TEXT_TYPE = Components.interfaces.nsIDOMNode.TEXT_NODE; + ////////////////////////////////////////////////////////////////////////////// // // Zotero.Annotate @@ -36,8 +38,7 @@ Zotero.Annotate = new function() { this.getPathForPoint = getPathForPoint; this.getPointForPath = getPointForPath; this.getPixelOffset = getPixelOffset; - - var textType = Components.interfaces.nsIDOMNode.TEXT_NODE; + this.normalizeRange = normalizeRange; /* * gets a path object, comprising an XPath, text node index, and offset, for @@ -48,14 +49,14 @@ Zotero.Annotate = new function() { var path = {parent:"", textNode:null, offset:(offset ? offset : null)}; - var lastWasTextNode = node.nodeType == textType; + var lastWasTextNode = node.nodeType == TEXT_TYPE; if(node.parentNode.getAttribute && node.parentNode.getAttribute("zotero")) { // if the selected point is inside a highlight node, add offsets of // preceding text nodes in this zotero node var sibling = node.previousSibling; while(sibling) { - if(sibling.nodeType == textType) path.offset += sibling.nodeValue.length; + if(sibling.nodeType == TEXT_TYPE) path.offset += sibling.nodeValue.length; sibling = sibling.previousSibling; } @@ -65,13 +66,16 @@ Zotero.Annotate = new function() { // if selected point is a zotero node, move it to the last character // of the previous node node = node.previousSibling; - if(node.nodeType == textType) { - offset = node.nodeValue.length; + if(node.nodeType == TEXT_TYPE) { + path.offset = node.nodeValue.length; + lastWasTextNode = true; } else { - offset = 0; + path.offset = 0; } } + lastWasTextNode = lastWasTextNode || node.nodeType == TEXT_TYPE; + if(lastWasTextNode) { path.textNode = 1; var sibling = node.previousSibling; @@ -81,7 +85,7 @@ Zotero.Annotate = new function() { var isZotero = undefined; if(sibling.getAttribute) isZotero = sibling.getAttribute("zotero"); - if(sibling.nodeType == textType || + if(sibling.nodeType == TEXT_TYPE || (isZotero == "highlight")) { // is a text node if(first == true) { @@ -89,7 +93,9 @@ Zotero.Annotate = new function() { if(sibling.getAttribute) { // get offset of all child nodes for each(var child in sibling.childNodes) { - if(child.nodeType == textType) path.offset += child.nodeValue.length; + if(child && child.nodeType == TEXT_TYPE) { + path.offset += child.nodeValue.length; + } } } else { path.offset += sibling.nodeValue.length; @@ -108,6 +114,10 @@ Zotero.Annotate = new function() { } node = node.parentNode; + } else if(path.offset != 0) { + for(; path.offset > 0 && node.nextSibling; path.offset--) { + node = node.nextSibling; + } } var doc = node.ownerDocument; @@ -121,9 +131,13 @@ Zotero.Annotate = new function() { } // don't add highlight nodes - var tag = node.tagName.toLowerCase(); - if(tag == "span") { - tag += "[not(@zotero)]"; + if(node.tagName) { + var tag = node.tagName.toLowerCase(); + if(tag == "span") { + tag += "[not(@zotero)]"; + } + } else { + var tag = node.nodeType; } path.parent = "/"+tag+"["+number+"]"+path.parent; @@ -167,7 +181,7 @@ Zotero.Annotate = new function() { var isZotero = undefined; if(point.node.getAttribute) isZotero = point.node.getAttribute("zotero"); - if(point.node.nodeType == textType || + if(point.node.nodeType == TEXT_TYPE || isZotero == "highlight") { if(!lastWasTextNode) { number++; @@ -198,7 +212,7 @@ Zotero.Annotate = new function() { var parentNode = point.node; point.node = point.node.firstChild; while(point.node) { - if(point.node.nodeType == textType) { + if(point.node.nodeType == TEXT_TYPE) { // break if end condition reached if(point.node.nodeValue.length >= point.offset) return point; // otherwise, continue subtracting offsets @@ -218,7 +232,7 @@ Zotero.Annotate = new function() { point.node = point.node.nextSibling; // if next node does not exist or is not a text node, this // point is invalid - if(!point.node || (point.node.nodeType != textType && + if(!point.node || (point.node.nodeType != TEXT_TYPE && (!point.node.getAttribute || !point.node.getAttribute("zotero")))) { Zotero.debug("Annotate: could not find point.offset "+point.offset+" for text node "+textNode+" of "+parent); return false; @@ -242,6 +256,30 @@ Zotero.Annotate = new function() { return [x, y]; } + + /* + * Sometimes, Firefox stupidly decides to use node offsets to reference + * nodes. This is a terrible idea, because Firefox then can't figure out + * when the node boundary matches another. This fixes it. + */ + function normalizeRange(range) { + for each(var type in ["start", "end"]) { + var container = range[type+"Container"]; + var offset = range[type+"Offset"]; + + if(container.nodeType != TEXT_TYPE && offset) { + for(; offset > 0 && container.nextSibling; offset--) { + container = container.nextSibling; + } + + if(type == "start") { + range.setStart(container, offset); + } else { + range.setEnd(container, offset); + } + } + } + } } ////////////////////////////////////////////////////////////////////////////// @@ -332,6 +370,7 @@ Zotero.Annotations.prototype.createHighlight = function(selectedRange) { return this.highlights[endIn]; } + Zotero.Annotate.normalizeRange(selectedRange); var highlight = new Zotero.Highlight(this); highlight.initWithRange(selectedRange); this.highlights.push(highlight); @@ -344,6 +383,8 @@ Zotero.Annotations.prototype.unhighlight = function(selectedRange) { var compareHighlight = this.highlights[i]; var compareRange = compareHighlight.range; + Zotero.Annotate.normalizeRange(compareRange); + var startToStart = compareRange.compareBoundaryPoints(Components.interfaces.nsIDOMRange.START_TO_START, selectedRange); var endToEnd = compareRange.compareBoundaryPoints(Components.interfaces.nsIDOMRange.END_TO_END, selectedRange); @@ -366,11 +407,14 @@ Zotero.Annotations.prototype.unhighlight = function(selectedRange) { selectEndPoint.offset == compareEndPoint.offset) { endToEnd = 0; } else { + // this will unhighlight the entire end compareHighlight.unhighlight(selectedRange.startContainer, selectedRange.startOffset, 2); - // need to use point references because they disregard highlights var newRange = this.document.createRange(); + + // need to use point references because they disregard highlights + var startPoint = Zotero.Annotate.getPointForPath(selectEndPoint.parent, selectEndPoint.textNode, selectEndPoint.offset, this.document, this.nsResolver); var endPoint = Zotero.Annotate.getPointForPath(compareEndPoint.parent, compareEndPoint.textNode, compareEndPoint.offset, @@ -456,9 +500,13 @@ Zotero.Annotations.prototype.load = function() { // load highlights var rows = Zotero.DB.query("SELECT * FROM highlights WHERE itemID = ?", [this.itemID]); for each(var row in rows) { - var highlight = new Zotero.Highlight(this); - highlight.initWithDBRow(row); - this.highlights.push(highlight); + try { + var highlight = new Zotero.Highlight(this); + highlight.initWithDBRow(row); + this.highlights.push(highlight); + } catch(e) { + Zotero.debug("Annotate: could not load highlight"); + } } } @@ -737,7 +785,6 @@ Zotero.Highlight = function(annotationsObj) { } Zotero.Highlight.prototype.initWithDBRow = function(row) { - Zotero.debug(row.startParent); var start = Zotero.Annotate.getPointForPath(row.startParent, row.startTextNode, row.startOffset, this.document, this.nsResolver); var end = Zotero.Annotate.getPointForPath(row.endParent, row.endTextNode, @@ -796,7 +843,7 @@ Zotero.Highlight.prototype.unhighlight = function(container, offset, mode) { this.range.setEnd(container, offset); } - for(var i in this.spans) { + for(var i=0; i