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

…hout it` (close DevExpress#1164)
LavrovArtem · Jun 2, 2017 · 43abe64 · 43abe64
1 parent d1854b7
commit 43abe64
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 7 deletions.
diff --git a/src/client/sandbox/code-instrumentation/properties/index.js b/src/client/sandbox/code-instrumentation/properties/index.js
@@ -117,6 +117,13 @@ export default class PropertyAccessorsInstrumentation extends SandboxBase {
     _createPropertyAccessors (window, document) {
         var storedDomain = '';
 
+        function hasElementProperty (el, prop) {
+            // NOTE: The first condition has been working just before the property has been assigned.
+            // We check the property via the hasOwnProperty function
+            // because the property is located into the element's prototype.
+            return el[prop] !== void 0 && !el.hasOwnProperty(prop);
+        }
+
         return {
             action: {
                 condition: el => domUtils.isDomElement(el) && domProcessor.isUrlAttr(el, 'action'),
@@ -276,7 +283,7 @@ export default class PropertyAccessorsInstrumentation extends SandboxBase {
             },
 
             innerHTML: {
-                condition: el => domUtils.isElementNode(el),
+                condition: el => domUtils.isElementNode(el) && hasElementProperty(el, 'innerHTML'),
 
                 get: el => {
                     if (domUtils.isScriptElement(el))
@@ -336,7 +343,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) && hasElementProperty(el, 'innerText'),
 
                 get: el => PropertyAccessorsInstrumentation.removeProcessingInstructions(el.innerText),
 
@@ -356,7 +363,7 @@ export default class PropertyAccessorsInstrumentation extends SandboxBase {
             },
 
             outerHTML: {
-                condition: el => typeof el.outerHTML === 'string' && domUtils.isElementNode(el),
+                condition: el => domUtils.isElementNode(el) && hasElementProperty(el, 'outerHTML'),
                 get:       el => cleanUpHtml(el.outerHTML, el.parentNode && el.parentNode.tagName),
 
                 set: (el, value) => {

diff --git a/test/client/fixtures/sandbox/code-instrumentation/getters-test.js b/test/client/fixtures/sandbox/code-instrumentation/getters-test.js
@@ -365,12 +365,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 () {
@@ -524,3 +524,48 @@ 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 svg             = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
+    var html            = '<a href="/path">link</a>';
+    var processedHtml   = processHtml(html);
+    var processedStyle  = styleProcessor.process('body {}', null, true);
+    var htmlPproperties = ['innerHTML', 'outerHTML'];
+    var textProperties  = ['innerText', 'text', 'textContent'];
+    var haveAsserts     = false;
+
+    htmlPproperties.forEach(function (property) {
+        var svgHasProperty = svg[property] !== void 0;
+
+        if (svgHasProperty)
+            return;
+
+        haveAsserts = true;
+
+        strictEqual(getProperty(svg, property), void 0);
+
+        setProperty(svg, property, html);
+
+        strictEqual(svg[property], html);
+
+        svg[property] = processedHtml;
+
+        strictEqual(getProperty(svg, property), processedHtml);
+    });
+
+    textProperties.forEach(function (property) {
+        var svgHasProperty = svg[property] !== void 0;
+
+        if (svgHasProperty)
+            return;
+
+        haveAsserts = true;
+
+        svg[property] = processedStyle;
+
+        strictEqual(getProperty(svg, property), processedStyle, property);
+    });
+
+    if (!haveAsserts)
+        expect(0);
+});