From 37a015eb8e756a72f5543ed459124fb512148359 Mon Sep 17 00:00:00 2001 From: Vilmos Ioo Date: Wed, 17 Sep 2014 13:57:58 +0100 Subject: [PATCH 1/4] Added support for ranges between element and text nodes --- src/models/cfi_generator.js | 42 +++++++++++++++++++++++ src/templates/cfi_library_template.js.erb | 3 ++ 2 files changed, 45 insertions(+) diff --git a/src/models/cfi_generator.js b/src/models/cfi_generator.js index bf2d972..d272749 100644 --- a/src/models/cfi_generator.js +++ b/src/models/cfi_generator.js @@ -82,6 +82,48 @@ EPUBcfi.Generator = { return commonCFIComponent.substring(1, commonCFIComponent.length) + "," + range1CFI + "," + range2CFI; }, + generateRangeComponent : function (rangeStartElement, startOffset, rangeEndElement, endOffset, classBlacklist, elementBlacklist, idBlacklist) { + if(rangeStartElement.nodeType === Node.ELEMENT_NODE && rangeEndElement.nodeType === Node.ELEMENT_NODE){ + return generator.generateElementRangeComponent(rangeStartElement, rangeEndElement, classBlacklist, elementBlacklist, idBlacklist); + } else if(rangeStartElement.nodeType === Node.TEXT_NODE && rangeEndElement.nodeType === Node.TEXT_NODE){ + return generator.generateCharOffsetRangeComponent(rangeStartElement, startOffset, rangeEndElement, endOffset, classBlacklist, elementBlacklist, idBlacklist); + } else { + var docRange, range1CFI, range1OffsetStep, range2CFI, range2OffsetStep; + + // Create a document range to find the common ancestor + docRange = document.createRange(); + docRange.setStart(rangeStartElement, startOffset); + docRange.setEnd(rangeEndElement, endOffset); + commonAncestor = docRange.commonAncestorContainer; + + if(rangeStartElement.nodeType === Node.ELEMENT_NODE){ + this.validateStartElement(rangeStartElement); + range1CFI = this.createCFIElementSteps($(rangeStartElement), commonAncestor, classBlacklist, elementBlacklist, idBlacklist); + } else { + this.validateStartTextNode(rangeStartElement); + // Generate terminating offset and range 1 + range1OffsetStep = this.createCFITextNodeStep($(rangeStartElement), startOffset, classBlacklist, elementBlacklist, idBlacklist); + range1CFI = this.createCFIElementSteps($(rangeStartElement).parent(), commonAncestor, classBlacklist, elementBlacklist, idBlacklist) + range1OffsetStep; + } + + if(rangeEndElement.nodeType === Node.ELEMENT_NODE){ + this.validateStartElement(rangeEndElement); + range2CFI = this.createCFIElementSteps($(rangeEndElement), commonAncestor, classBlacklist, elementBlacklist, idBlacklist); + } else { + this.validateStartTextNode(rangeEndElement); + // Generate terminating offset and range 1 + range2OffsetStep = this.createCFITextNodeStep($(rangeEndElement), endOffset, classBlacklist, elementBlacklist, idBlacklist); + range2CFI = this.createCFIElementSteps($(rangeEndElement).parent(), commonAncestor, classBlacklist, elementBlacklist, idBlacklist) + range2OffsetStep; + } + + // Generate shared component + commonCFIComponent = this.createCFIElementSteps($(commonAncestor), "html", classBlacklist, elementBlacklist, idBlacklist); + + // Return the result + return commonCFIComponent.substring(1, commonCFIComponent.length) + "," + range1CFI + "," + range2CFI; + } + }, + // Description: Generates a character offset CFI // Arguments: The text node that contains the offset referenced by the cfi, the offset value, the name of the // content document that contains the text node, the package document for this EPUB. diff --git a/src/templates/cfi_library_template.js.erb b/src/templates/cfi_library_template.js.erb index 2a4e9df..1f55c6d 100644 --- a/src/templates/cfi_library_template.js.erb +++ b/src/templates/cfi_library_template.js.erb @@ -77,6 +77,9 @@ }, generateElementRangeComponent : function (rangeStartElement, rangeEndElement, classBlacklist, elementBlacklist, idBlacklist) { return generator.generateElementRangeComponent(rangeStartElement, rangeEndElement, classBlacklist, elementBlacklist, idBlacklist); + }, + generateRangeComponent : function (rangeStartElement, startOffset, rangeEndElement, endOffset, classBlacklist, elementBlacklist, idBlacklist) { + return generator.generateRangeComponent(rangeStartElement, startOffset, rangeEndElement, endOffset, classBlacklist, elementBlacklist, idBlacklist); } }; From 7a88de211dcf2167e4f9144bc5978014a84d459b Mon Sep 17 00:00:00 2001 From: Vilmos Ioo Date: Thu, 18 Sep 2014 16:06:31 +0100 Subject: [PATCH 2/4] Added text-element range test. --- .../javascripts/models/cfi_generation_spec.js | 25 +++++++++++++++++++ src/models/cfi_generator.js | 4 +-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/spec/javascripts/models/cfi_generation_spec.js b/spec/javascripts/models/cfi_generation_spec.js index 4e5051a..5aca57f 100644 --- a/spec/javascripts/models/cfi_generation_spec.js +++ b/spec/javascripts/models/cfi_generation_spec.js @@ -26,6 +26,31 @@ describe("CFI GENERATOR", function () { expect(generatedCFI).toEqual("/2[startParent]/2"); }); + it("can generate a range component between a text node and an element node", function () { + + var dom = + "" + + "
" + + "
" + + "
" + + "
" + + "textnode1" + + "
" + + "textNode2" + + "
" + + "
" + + "
" + + "
" + + ""; + var $dom = $((new window.DOMParser).parseFromString(dom, "text/xml")); + + var $startElement1 = $($('#startParent', $dom).contents()[1]); + var $startElement2 = $($('#startParent', $dom).contents()[3]); + var generatedCFI = EPUBcfi.Generator.generateRangeComponent($startElement1[0], 1, $startElement2[0], 0); + + expect(generatedCFI).toEqual("/4/2[startParent],/1:1,/3:0"); + }); + it("can generate an element range CFI for a node with a period in the ID", function () { var dom = diff --git a/src/models/cfi_generator.js b/src/models/cfi_generator.js index d272749..574bbb0 100644 --- a/src/models/cfi_generator.js +++ b/src/models/cfi_generator.js @@ -84,9 +84,9 @@ EPUBcfi.Generator = { generateRangeComponent : function (rangeStartElement, startOffset, rangeEndElement, endOffset, classBlacklist, elementBlacklist, idBlacklist) { if(rangeStartElement.nodeType === Node.ELEMENT_NODE && rangeEndElement.nodeType === Node.ELEMENT_NODE){ - return generator.generateElementRangeComponent(rangeStartElement, rangeEndElement, classBlacklist, elementBlacklist, idBlacklist); + return this.generateElementRangeComponent(rangeStartElement, rangeEndElement, classBlacklist, elementBlacklist, idBlacklist); } else if(rangeStartElement.nodeType === Node.TEXT_NODE && rangeEndElement.nodeType === Node.TEXT_NODE){ - return generator.generateCharOffsetRangeComponent(rangeStartElement, startOffset, rangeEndElement, endOffset, classBlacklist, elementBlacklist, idBlacklist); + return this.generateCharOffsetRangeComponent(rangeStartElement, startOffset, rangeEndElement, endOffset, classBlacklist, elementBlacklist, idBlacklist); } else { var docRange, range1CFI, range1OffsetStep, range2CFI, range2OffsetStep; From 19880fcd11790eb42e2f2cd920c2f507fc37aa6e Mon Sep 17 00:00:00 2001 From: Vilmos Ioo Date: Fri, 19 Sep 2014 10:59:18 +0100 Subject: [PATCH 3/4] Added unit tests --- .../javascripts/models/cfi_generation_spec.js | 32 +++++++++++++++---- src/models/cfi_generator.js | 20 +++++++++--- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/spec/javascripts/models/cfi_generation_spec.js b/spec/javascripts/models/cfi_generation_spec.js index 5aca57f..6bd1930 100644 --- a/spec/javascripts/models/cfi_generation_spec.js +++ b/spec/javascripts/models/cfi_generation_spec.js @@ -33,22 +33,43 @@ describe("CFI GENERATOR", function () { + "
" + "
" + "
" + + "textnode" + "
" + "textnode1" + "
" - + "textNode2" - + "
" + "
" + "
" + "
" + ""; var $dom = $((new window.DOMParser).parseFromString(dom, "text/xml")); - var $startElement1 = $($('#startParent', $dom).contents()[1]); - var $startElement2 = $($('#startParent', $dom).contents()[3]); + var $startElement1 = $($('#startParent', $dom).contents()[0]); + var $startElement2 = $($('#startParent', $dom).contents()[1]); var generatedCFI = EPUBcfi.Generator.generateRangeComponent($startElement1[0], 1, $startElement2[0], 0); + expect(generatedCFI).toEqual("/4/2[startParent],/1:1,/2"); + }); + + it("can generate a range component between an element node and a text node", function () { + + var dom = + "" + + "
" + + "
" + + "
" + + "textnode" + + "
" + + "textnode1" + + "
" + + "
" + + "
" + + "
" + + ""; + var $dom = $((new window.DOMParser).parseFromString(dom, "text/xml")); - expect(generatedCFI).toEqual("/4/2[startParent],/1:1,/3:0"); + var $startElement1 = $($('#startParent', $dom).contents()[1]); + var $startElement2 = $($('#startParent', $dom).contents()[2]); + var generatedCFI = EPUBcfi.Generator.generateRangeComponent($startElement1[0], 0, $startElement2[0], 1); + expect(generatedCFI).toEqual("/4/2[startParent],/2,/3:1"); }); it("can generate an element range CFI for a node with a period in the ID", function () { @@ -397,7 +418,6 @@ describe("CFI GENERATOR", function () { ); - console.log(generatedCFI); expect(generatedCFI).toEqual("/4/2[startParent],/1:14,/1:18"); }); diff --git a/src/models/cfi_generator.js b/src/models/cfi_generator.js index 574bbb0..0f01c5e 100644 --- a/src/models/cfi_generator.js +++ b/src/models/cfi_generator.js @@ -88,7 +88,11 @@ EPUBcfi.Generator = { } else if(rangeStartElement.nodeType === Node.TEXT_NODE && rangeEndElement.nodeType === Node.TEXT_NODE){ return this.generateCharOffsetRangeComponent(rangeStartElement, startOffset, rangeEndElement, endOffset, classBlacklist, elementBlacklist, idBlacklist); } else { - var docRange, range1CFI, range1OffsetStep, range2CFI, range2OffsetStep; + var docRange; + var range1CFI; + var range1OffsetStep; + var range2CFI; + var range2OffsetStep; // Create a document range to find the common ancestor docRange = document.createRange(); @@ -103,7 +107,11 @@ EPUBcfi.Generator = { this.validateStartTextNode(rangeStartElement); // Generate terminating offset and range 1 range1OffsetStep = this.createCFITextNodeStep($(rangeStartElement), startOffset, classBlacklist, elementBlacklist, idBlacklist); - range1CFI = this.createCFIElementSteps($(rangeStartElement).parent(), commonAncestor, classBlacklist, elementBlacklist, idBlacklist) + range1OffsetStep; + if($(rangeStartElement).parent().is(commonAncestor)){ + range1CFI = range1OffsetStep; + } else { + range1CFI = this.createCFIElementSteps($(rangeStartElement).parent(), commonAncestor, classBlacklist, elementBlacklist, idBlacklist) + range1OffsetStep; + } } if(rangeEndElement.nodeType === Node.ELEMENT_NODE){ @@ -111,9 +119,13 @@ EPUBcfi.Generator = { range2CFI = this.createCFIElementSteps($(rangeEndElement), commonAncestor, classBlacklist, elementBlacklist, idBlacklist); } else { this.validateStartTextNode(rangeEndElement); - // Generate terminating offset and range 1 + // Generate terminating offset and range 2 range2OffsetStep = this.createCFITextNodeStep($(rangeEndElement), endOffset, classBlacklist, elementBlacklist, idBlacklist); - range2CFI = this.createCFIElementSteps($(rangeEndElement).parent(), commonAncestor, classBlacklist, elementBlacklist, idBlacklist) + range2OffsetStep; + if($(rangeEndElement).parent().is(commonAncestor)){ + range2CFI = range2OffsetStep; + } else { + range2CFI = this.createCFIElementSteps($(rangeEndElement).parent(), commonAncestor, classBlacklist, elementBlacklist, idBlacklist) + range2OffsetStep; + } } // Generate shared component From 2dd4c8e23fadf95cfea7d2e647c8c37f2aacef76 Mon Sep 17 00:00:00 2001 From: Vilmos Ioo Date: Fri, 19 Sep 2014 11:04:05 +0100 Subject: [PATCH 4/4] Added edge case scenario --- .../javascripts/models/cfi_generation_spec.js | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/spec/javascripts/models/cfi_generation_spec.js b/spec/javascripts/models/cfi_generation_spec.js index 6bd1930..1f78a1c 100644 --- a/spec/javascripts/models/cfi_generation_spec.js +++ b/spec/javascripts/models/cfi_generation_spec.js @@ -72,6 +72,30 @@ describe("CFI GENERATOR", function () { expect(generatedCFI).toEqual("/4/2[startParent],/2,/3:1"); }); + it("can generate a range component between an element node and a text node with different parents", function () { + + var dom = + "" + + "
" + + "
" + + "
" + + "textnode" + + "
" + + "textnode1" + + "
" + + "
" + + "
" + + "
" + + ""; + var $dom = $((new window.DOMParser).parseFromString(dom, "text/xml")); + + var $startElement1 = $($('#startParent', $dom).contents()[0]); + var $startElement2 = $($('#end', $dom)[0]); + var generatedCFI = EPUBcfi.Generator.generateRangeComponent($startElement1[0], 1, $startElement2[0], 0); + expect(generatedCFI).toEqual("/2,/4/2[startParent]/1:1,/6[end]"); + }); + + it("can generate an element range CFI for a node with a period in the ID", function () { var dom =