DLynch has uploaded a new change for review.
https://gerrit.wikimedia.org/r/266265
Change subject: Remove uses of plain .contentEditable
......................................................................
Remove uses of plain .contentEditable
Browsers handle the inheritance via isContentEditable, so we don't have to
think about it. Add a helper method so checks don't have to think about
text nodes.
Bug: T116214
Change-Id: Ib7beda39e9195c77d29faf8120c89c053b6f2b90
---
M src/ce/ve.ce.Surface.js
M src/ce/ve.ce.View.js
M src/ve.utils.js
3 files changed, 18 insertions(+), 12 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor
refs/changes/65/266265/1
diff --git a/src/ce/ve.ce.Surface.js b/src/ce/ve.ce.Surface.js
index 9fa264d..92e1dc4 100644
--- a/src/ce/ve.ce.Surface.js
+++ b/src/ce/ve.ce.Surface.js
@@ -1155,7 +1155,7 @@
* @return {ve.ce.Node|null} node, or null if not in a focusable node
*/
function getSurroundingFocusableNode( node, offset, direction ) {
- var focusNode, $focusNode;
+ var focusNode;
if ( node.nodeType === Node.TEXT_NODE ) {
focusNode = node;
} else if ( direction > 0 && offset < node.childNodes.length ) {
@@ -1166,13 +1166,12 @@
focusNode = node;
}
- $focusNode = $( focusNode );
// If the first ancestor with contenteditable set is ce=true,
then we are allowed
// to be inside this focusable node (e.g. editing a table cell
or caption)
- if ( $focusNode.closest( '[contenteditable]' ).prop(
'contenteditable' ) === 'true' ) {
+ if ( ve.isContentEditable( focusNode ) ) {
return null;
}
- return $focusNode.closest( '.ve-ce-focusableNode,
.ve-ce-tableNode' ).data( 'view' ) || null;
+ return $( focusNode ).closest( '.ve-ce-focusableNode,
.ve-ce-tableNode' ).data( 'view' ) || null;
}
/**
diff --git a/src/ce/ve.ce.View.js b/src/ce/ve.ce.View.js
index 81099b4..b893557 100644
--- a/src/ce/ve.ce.View.js
+++ b/src/ce/ve.ce.View.js
@@ -171,11 +171,7 @@
* @return {boolean} Node is inside a contentEditable node
*/
ve.ce.View.prototype.isInContentEditable = function () {
- var node = this.$element[ 0 ].parentNode;
- while ( node && node.contentEditable === 'inherit' ) {
- node = node.parentNode;
- }
- return !!( node && node.contentEditable === 'true' );
+ return ve.isContentEditable( this.$element[ 0 ].parentNode );
};
/**
diff --git a/src/ve.utils.js b/src/ve.utils.js
index 7a15804..0106125 100644
--- a/src/ve.utils.js
+++ b/src/ve.utils.js
@@ -701,6 +701,19 @@
};
/**
+ * Check whether a given node is contentEditable
+ *
+ * Handles 'inherit', via checking isContentEditable. Knows to check for text
+ * nodes, and will return whether the text node's parent is contentEditable.
+ *
+ * @param {HTMLElement} node Node to check contenteditable status of
+ * @return {boolean} Node is contenteditable
+ */
+ve.isContentEditable = function ( node ) {
+ return ( node.nodeType === Node.TEXT_NODE ? node.parentNode : node
).isContentEditable;
+};
+
+/**
* Create an HTMLDocument from an HTML string.
*
* The html parameter is supposed to be a full HTML document with a doctype
and an `<html>` tag.
@@ -1610,9 +1623,7 @@
if ( ve.isVoidElement( node ) ) {
return true;
}
- // We don't need to check whether the ancestor-nearest contenteditable
tag is
- // false, because if so then there can be no adjacent cursor.
- return node.contentEditable === 'false';
+ return !ve.isContentEditable( node );
};
/**
--
To view, visit https://gerrit.wikimedia.org/r/266265
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib7beda39e9195c77d29faf8120c89c053b6f2b90
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: DLynch <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
