fix `We don't need to process the innerHTML property for elements wit…

…hout it` (close DevExpress#1164)
LavrovArtem · Jun 6, 2017 · 6692378 · 6692378
1 parent 2218d58
commit 6692378
Show file tree

Hide file tree

Showing 3 changed files with 92 additions and 8 deletions.
diff --git a/src/client/sandbox/code-instrumentation/properties/index.js b/src/client/sandbox/code-instrumentation/properties/index.js
@@ -266,7 +266,7 @@ export default class PropertyAccessorsInstrumentation extends SandboxBase {
             },
 
             innerHTML: {
-                condition: el => domUtils.isElementNode(el),
+                condition: el => domUtils.isElementNode(el) && domUtils.hasElementProperty(el, 'innerHTML'),
 
                 get: el => {
                     if (domUtils.isScriptElement(el))
@@ -335,7 +335,7 @@ export default class PropertyAccessorsInstrumentation extends SandboxBase {
 
             innerText: {
                 // NOTE: http://caniuse.com/#search=Node.innerText
-                condition: el => typeof el.innerText === 'string' && domUtils.isElementNode(el),
+                condition: el => domUtils.isElementNode(el) && domUtils.hasElementProperty(el, 'innerText'),
 
                 get: el => PropertyAccessorsInstrumentation.removeProcessingInstructions(el.innerText),
 
@@ -355,7 +355,7 @@ export default class PropertyAccessorsInstrumentation extends SandboxBase {
             },
 
             outerHTML: {
-                condition: el => typeof el.outerHTML === 'string' && domUtils.isElementNode(el),
+                condition: el => domUtils.isElementNode(el) && domUtils.hasElementProperty(el, 'outerHTML'),
                 get:       el => cleanUpHtml(el.outerHTML, el.parentNode && el.parentNode.tagName),
 
                 set: (el, value) => {
@@ -588,7 +588,7 @@ export default class PropertyAccessorsInstrumentation extends SandboxBase {
             },
 
             textContent: {
-                condition: el => domUtils.isElementNode(el),
+                condition: el => domUtils.isElementNode(el) && domUtils.hasElementProperty(el, 'textContent'),
 
                 get: el => PropertyAccessorsInstrumentation.removeProcessingInstructions(el.textContent),
 

diff --git a/src/client/utils/dom.js b/src/client/utils/dom.js
@@ -16,6 +16,20 @@ const arraySlice = Array.prototype.slice;
 
 var scrollbarSize = null;
 
+const ELEMENT_TEXT_PROPERTIES = ['innerHTML', 'outerHTML', 'innerText', 'textContent', 'text'];
+
+function getAttrValues (element) {
+    var values = {};
+
+    for (var property of ELEMENT_TEXT_PROPERTIES)
+        values[property] = element[property];
+
+    return values;
+}
+
+const DEFAULT_SVG_ELEMENT_TEXT_ATTR_VALUES  = getAttrValues(nativeMethods.createElementNS.call(document, 'http://www.w3.org/2000/svg', 'svg'));
+const DEFAULT_HTML_ELEMENT_TEXT_ATTR_VALUES = getAttrValues(nativeMethods.createElement.call(document, 'div'));
+
 const NATIVE_ELEMENT_PROTOTYPE_STRINGS = [
     instanceToString(nativeMethods.elementClass.prototype),
     instanceToString(Object.getPrototypeOf(nativeMethods.elementClass.prototype))
@@ -761,3 +775,13 @@ export function getFileInputs (el) {
 export function getIframes (el) {
     return isIframeElement(el) ? [el] : getNativeQuerySelectorAll(el).call(el, 'iframe,frame');
 }
+
+export function hasElementProperty (el, prop) {
+    if (el[prop] === void 0)
+        return false;
+
+    if (isSVGElement(el))
+        return DEFAULT_SVG_ELEMENT_TEXT_ATTR_VALUES[prop] !== void 0;
+
+    return DEFAULT_HTML_ELEMENT_TEXT_ATTR_VALUES[prop] !== void 0;
+}
diff --git a/test/client/fixtures/sandbox/code-instrumentation/getters-test.js b/test/client/fixtures/sandbox/code-instrumentation/getters-test.js
@@ -366,12 +366,12 @@ test('get script body (T296958) (GH-183)', function () {
     notEqual(script.textContent, scriptCode);
     strictEqual(script.textContent.replace(/\s/g, ''), processedScriptCode.replace(/\s/g, ''));
     strictEqual(cleanedScriptCode.indexOf(INTERNAL_PROPS.processDomMethodName), -1);
-    strictEqual(getProperty(script, 'text'), cleanedScriptCode);
-    strictEqual(getProperty(script, 'textContent'), cleanedScriptCode);
-    strictEqual(getProperty(script, 'innerHTML'), cleanedScriptCode);
+    strictEqual(getProperty(script, 'text'), cleanedScriptCode, 'text');
+    strictEqual(getProperty(script, 'textContent'), cleanedScriptCode, 'textContent');
+    strictEqual(getProperty(script, 'innerHTML'), cleanedScriptCode, 'innerHTML');
 
     if (typeof script.innerText === 'string')
-        strictEqual(getProperty(script, 'innerText').replace(/\s/g, ''), cleanedScriptCode.replace(/\s/g, ''));
+        strictEqual(getProperty(script, 'innerText').replace(/\s/g, ''), cleanedScriptCode.replace(/\s/g, ''), 'innerText');
 });
 
 test('the getAttributesProperty function should work correctly if Function.prototype.bind is removed (GH-359)', function () {
@@ -525,3 +525,63 @@ test('we should clean up html and remove extra namespaces from svg (GH-1083)', f
 
     strictEqual(getProperty(div, 'innerHTML'), nativeDiv.innerHTML);
 });
+
+test('we should not process element\'s properties if they do not exist (GH-1164)', function () {
+    var div             = document.createElement('div');
+    var svg             = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
+    var html            = '<a href="/path">link</a>';
+    var processedHtml   = processHtml(html);
+    var style           = 'body {}';
+    var processedStyle  = styleProcessor.process(style, null, true);
+    var textProperties  = ['innerText', 'text', 'textContent'];
+
+    div.appendChild(svg);
+
+    var svgHasInnerHTML = svg.innerHTML !== void 0;
+
+    if (svgHasInnerHTML)
+        strictEqual(getProperty(svg, 'innerHTML'), '');
+    else
+        strictEqual(getProperty(svg, 'innerHTML'), void 0);
+
+    setProperty(svg, 'innerHTML', html);
+
+    if (svgHasInnerHTML)
+        strictEqual(svg.innerHTML, processedHtml);
+    else
+        strictEqual(svg.innerHTML, html);
+
+    svg.innerHTML = processedHtml;
+
+    if (svgHasInnerHTML)
+        strictEqual(getProperty(svg, 'innerHTML'), html);
+    else
+        strictEqual(getProperty(svg, 'innerHTML'), processedHtml);
+
+    svg.innerHTML = '';
+
+    var svgHasOuterHTML = svg.outerHTML !== void 0;
+
+    if (svgHasOuterHTML)
+        strictEqual(getProperty(svg, 'outerHTML'), svg.outerHTML);
+    else
+        strictEqual(getProperty(svg, 'outerHTML'), void 0);
+
+    setProperty(svg, 'outerHTML', html);
+
+    if (svgHasOuterHTML)
+        strictEqual(div.innerHTML, processedHtml);
+    else
+        strictEqual(svg.outerHTML, html);
+
+    textProperties.forEach(function (property) {
+        var svgHasProperty = svg[property] !== void 0;
+
+        svg[property] = processedStyle;
+
+        if (svgHasProperty)
+            strictEqual(getProperty(svg, property), style, property);
+        else
+            strictEqual(getProperty(svg, property), processedStyle, property);
+    });
+});