Skip to content
This repository was archived by the owner on May 7, 2025. It is now read-only.

Readium shared issue #386, added helper function for node.parentElement #387

Open
einarstubhaug wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Readium shared issue #386, added helper function for node.parentElement #387

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b33980d
Readium shared issue #386, added helper function for node.parentEleme…
einarstubhaug May 16, 2017
2efa5f5
element.className fails for svg, use element.getAttribute("class") in…
Jun 7, 2017
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
28 changes: 25 additions & 3 deletions js/views/cfi_navigation_logic.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,28 @@ var CfiNavigationLogic = function(options) {

}

/**
* @private
* Helper function because of #386, node.parentElement returns null for text nodes on IE
*
* @returns { node } parentElement
*/
function getParentElement(node) {
Copy link
Copy Markdown
Member

@danielweck danielweck May 21, 2017

Choose a reason for hiding this comment

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

Just a note, rangy-core's implementation is:

    function parentElement(node) {
        var parent = node.parentNode;
        return (parent.nodeType == 1) ? parent : null;
    }

if (node.parentElement) {
return node.parentElement;
Copy link
Copy Markdown
Member

@danielweck danielweck May 21, 2017

Choose a reason for hiding this comment

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

Note to self: node.parentElement !== textRange.parentElement() (the latter is a function).

} else {
// IE 10 & IE 11
if (node.parentNode) {
if (node.parentNode.nodeType === 1) {
return node.parentNode;
} else {
// walk up dom
return getParentElement(node.parentNode)
}
}
}
}

/**
* @private
* Retrieves _current_ full width of a column (including its gap)
Expand Down Expand Up @@ -1299,7 +1321,7 @@ var CfiNavigationLogic = function(options) {
_.each($elements, function ($node) {
var node = $node[0];
var isTextNode = (node.nodeType === Node.TEXT_NODE);
var element = isTextNode ? node.parentElement : node;
var element = isTextNode ? getParentElement(node) : node;
Copy link
Copy Markdown
Member

@danielweck danielweck May 21, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

There are other uses of node.parentElement, notably plugins/highlights/manager.js getAnnotationMidpoints()
https://github.com/readium/readium-shared-js/blob/develop/plugins/highlights/manager.js#L304
I also note the use of jQuery's $node.parent() in many places in ReadiumJS, notably:

//figure out a better way to get the html parent from an element..
var $html = $element.parent();

https://github.com/readium/readium-shared-js/blob/develop/plugins/highlights/manager.js#L322

@jccr

var visibilityPercentage = checkVisibilityByRectangles(
$node, true, visibleContentOffsets, frameDimensions);

Expand Down Expand Up @@ -1365,7 +1387,7 @@ var CfiNavigationLogic = function(options) {

function isElementBlacklisted(element) {
var isBlacklisted = false;
var classAttribute = element.className;
var classAttribute = element.getAttribute("class");
var classList = classAttribute ? classAttribute.split(' ') : [];
var id = element.id;

Expand Down Expand Up @@ -1410,7 +1432,7 @@ var CfiNavigationLogic = function(options) {
while ((node = nodeIterator.nextNode())) {
var isLeafNode = node.nodeType === Node.ELEMENT_NODE && !node.childElementCount && !isValidTextNodeContent(node.textContent);
if (isLeafNode || isValidTextNode(node)){
var element = (node.nodeType === Node.TEXT_NODE) ? node.parentElement : node;
var element = (node.nodeType === Node.TEXT_NODE) ? getParentElement(node) : node;
if (!isElementBlacklisted(element)) {
$leafNodeElements.push($(node));
}
Expand Down