Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Range normalization fix #238

Merged
merged 3 commits into from
Jul 16, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 42 additions & 31 deletions src/range.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -133,42 +133,53 @@ class Range.BrowserRange
@tainted = true

r = {}
nr = {}

for p in ['start', 'end']
node = this[p + 'Container']
offset = this[p + 'Offset']

if node.nodeType is Node.ELEMENT_NODE
# Get specified node.
it = node.childNodes[offset]
# If it doesn't exist, that means we need the end of the
# previous one.
node = it or node.childNodes[offset - 1]

# if node doesn't have any children, it's a <br> or <hr> or
# other self-closing tag, and we actually want the textNode
# that ends just before it
if node.nodeType is Node.ELEMENT_NODE and not node.firstChild
it = null # null out ref to node so offset is correctly calculated below.
node = node.previousSibling

while node.nodeType isnt Node.TEXT_NODE
node = node.firstChild

offset = if it then 0 else node.nodeValue.length

r[p] = node
r[p + 'Offset'] = offset

# Look at the start
if @startContainer.nodeType is Node.ELEMENT_NODE
# We are dealing with element nodes
r.start = Util.getFirstTextNodeNotBefore @startContainer.childNodes[@startOffset]
r.startOffset = 0
else
# We are dealing with simple text nodes
r.start = @startContainer
r.startOffset = @startOffset

# Look at the end
if @endContainer.nodeType is Node.ELEMENT_NODE
# Get specified node.
node = @endContainer.childNodes[@endOffset]

if node? # Does that node exist?
# Look for a text node either at the immediate beginning of node
n = node
while n? and (n.nodeType isnt Node.TEXT_NODE)
n = n.firstChild
if n? # Did we find a text node at the start of this element?
r.end = n
r.endOffset = 0

unless r.end?
# We need to find a text node in the previous node.
node = @endContainer.childNodes[@endOffset - 1]
r.end = Util.getLastTextNodeUpTo node
r.endOffset = r.end.nodeValue.length

else # We are dealing with simple text nodes
r.end = @endContainer
r.endOffset = @endOffset

# We have collected the initial data.

# Now let's start to slice & dice the text elements!
nr = {}
nr.start = if r.startOffset > 0 then r.start.splitText(r.startOffset) else r.start

if r.start is r.end
if (r.endOffset - r.startOffset) < nr.start.nodeValue.length
if r.start is r.end # is the whole selection inside one text element ?
if nr.start.nodeValue.length > (r.endOffset - r.startOffset)
nr.start.splitText(r.endOffset - r.startOffset)
nr.end = nr.start
else
if r.endOffset < r.end.nodeValue.length
else # no, the end of the selection is in a separate text element
if r.end.nodeValue.length > r.endOffset# does the end need to be cut?
r.end.splitText(r.endOffset)
nr.end = r.end

Expand Down
50 changes: 50 additions & 0 deletions src/util.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,18 @@ Util.flatten = (array) ->

flatten(array)

# Public: decides whether node A is an ancestor of node B.
#
# This function purposefully ignores the native browser function for this,
# because it acts weird in PhantomJS.
# Issue: https://github.com/ariya/phantomjs/issues/11479
Util.contains = (parent, child) ->
node = child
while node?
if node is parent then return true
node = node.parentNode
return false

# Public: Finds all text nodes within the elements in the current collection.
#
# Returns a new jQuery collection of text nodes.
Expand All @@ -59,6 +71,44 @@ Util.getTextNodes = (jq) ->

jq.map -> Util.flatten(getTextNodes(this))

# Public: determine the last text node inside or before the given node
Util.getLastTextNodeUpTo = (n) ->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might change these to "getPreviousTextNode" and "getNextTextNode". Maybe clearer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the proposed new names would be clearer, since "previous" sounds like we can not accept the current node, while in reality, we we can. So it would be a little bit misleading. (The same goes with next, too.)

switch n.nodeType
when Node.TEXT_NODE
return n # We have found our text node.
when Node.ELEMENT_NODE
# This is an element, we need to dig in
if n.lastChild? # Does it have children at all?
result = Util.getLastTextNodeUpTo n.lastChild
if result? then return result
else
# Not a text node, and not an element node.
# Could not find a text node in current node, go backwards
n = n.previousSibling
if n?
Util.getLastTextNodeUpTo n
else
null

# Public: determine the first text node in or after the given jQuery node.
Util.getFirstTextNodeNotBefore = (n) ->
switch n.nodeType
when Node.TEXT_NODE
return n # We have found our text node.
when Node.ELEMENT_NODE
# This is an element, we need to dig in
if n.firstChild? # Does it have children at all?
result = Util.getFirstTextNodeNotBefore n.firstChild
if result? then return result
else
# Not a text or an element node.
# Could not find a text node in current node, go forward
n = n.nextSibling
if n?
Util.getFirstTextNodeNotBefore n
else
null

Util.xpathFromNode = (el, relativeRoot) ->
try
result = simpleXPathJQuery.call el, relativeRoot
Expand Down
2 changes: 1 addition & 1 deletion test/helpers/spec_helper.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class this.MockSelection
@description = data[5]

@commonAncestor = @startContainer
while not @commonAncestor.contains @endContainer
while not Util.contains(@commonAncestor, @endContainer)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can temporarily revert this one-liner change to see errors caused by the problematic .contains implementation by PhantomJS.
(Some of the test cases will fail because of it - but only with PhantomJS, if you run it in the browser, it will still pass.)

@commonAncestor = @commonAncestor.parentNode
@commonAncestorXPath = Util.xpathFromNode($(@commonAncestor))[0]

Expand Down