HTMLElement.innerText implementation is now closer to the spec

HtmlUnit · May 15, 2021 · 3636f75 · 3636f75
1 parent 13e4551
commit 3636f75
Show file tree

Hide file tree

Showing 7 changed files with 107 additions and 31 deletions.
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
@@ -8,6 +8,9 @@
 
     <body>
         <release version="2.50.0" date="Mai xx, 2021" description="Bugfixes, Firefox 88, Chrome/Edge 89">
+            <action type="fix" dev="rbri">
+                HTMLElement.innerText implementation is now closer to the spec.
+            </action>
             <action type="fix" dev="RhinoTeam">
                 core-js: Optimization of the + operator.
             </action>

diff --git a/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlElement.java b/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlElement.java
@@ -46,7 +46,6 @@
 import com.gargoylesoftware.htmlunit.WebAssert;
 import com.gargoylesoftware.htmlunit.WebClient;
 import com.gargoylesoftware.htmlunit.html.impl.SelectableTextInput;
-import com.gargoylesoftware.htmlunit.html.serializer.HtmlSerializerInnerOuterText;
 import com.gargoylesoftware.htmlunit.javascript.host.dom.Document;
 import com.gargoylesoftware.htmlunit.javascript.host.dom.MutationObserver;
 import com.gargoylesoftware.htmlunit.javascript.host.event.Event;
@@ -1400,12 +1399,4 @@ public DomNode cloneNode(final boolean deep) {
 
         return newnode;
     }
-
-    /**
-     * @return a the innterText representation of this element
-     */
-    public String getInnerText() {
-        final HtmlSerializerInnerOuterText ser = new HtmlSerializerInnerOuterText();
-        return ser.asText(this);
-    }
 }
diff --git a/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/html/HTMLElement.java b/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/html/HTMLElement.java
@@ -43,6 +43,7 @@
 import java.util.regex.Pattern;
 
 import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.xml.sax.helpers.AttributesImpl;
@@ -65,6 +66,7 @@
 import com.gargoylesoftware.htmlunit.html.HtmlBig;
 import com.gargoylesoftware.htmlunit.html.HtmlBody;
 import com.gargoylesoftware.htmlunit.html.HtmlBold;
+import com.gargoylesoftware.htmlunit.html.HtmlBreak;
 import com.gargoylesoftware.htmlunit.html.HtmlCenter;
 import com.gargoylesoftware.htmlunit.html.HtmlCitation;
 import com.gargoylesoftware.htmlunit.html.HtmlCode;
@@ -110,6 +112,7 @@
 import com.gargoylesoftware.htmlunit.html.HtmlUnderlined;
 import com.gargoylesoftware.htmlunit.html.HtmlVariable;
 import com.gargoylesoftware.htmlunit.html.HtmlWordBreak;
+import com.gargoylesoftware.htmlunit.html.serializer.HtmlSerializerInnerOuterText;
 import com.gargoylesoftware.htmlunit.javascript.background.BackgroundJavaScriptFactory;
 import com.gargoylesoftware.htmlunit.javascript.background.JavaScriptJob;
 import com.gargoylesoftware.htmlunit.javascript.configuration.JsxClass;
@@ -715,15 +718,18 @@ public Object insertAdjacentElement(final String where, final Object insertedEle
 
     /**
      * Gets the innerText attribute.
+     * (see https://html.spec.whatwg.org/multipage/dom.html#the-innertext-idl-attribute)
      * @return the contents of this node as text
      */
     @JsxGetter
     public String getInnerText() {
-        return getDomNodeOrDie().getInnerText();
+        final HtmlSerializerInnerOuterText ser = new HtmlSerializerInnerOuterText();
+        return ser.asText(this.getDomNodeOrDie());
     }
 
     /**
      * Replaces all child elements of this element with the supplied text value.
+     * (see https://html.spec.whatwg.org/multipage/dom.html#the-innertext-idl-attribute)
      * @param value the new value for the contents of this element
      */
     @JsxSetter
@@ -735,20 +741,19 @@ public void setInnerText(final Object value) {
         else {
             valueString = Context.toString(value);
         }
-        setInnerTextImpl(valueString);
-    }
 
-    /**
-     * The worker for setInnerText.
-     * @param value the new value for the contents of this node
-     */
-    protected void setInnerTextImpl(final String value) {
         final DomNode domNode = getDomNodeOrDie();
-
+        final SgmlPage page = domNode.getPage();
         domNode.removeAllChildren();
 
-        if (value != null && !value.isEmpty()) {
-            domNode.appendChild(new DomText(domNode.getPage(), value));
+        if (StringUtils.isNotEmpty(valueString)) {
+            final String[] parts = valueString.split("\\r?\\n");
+            for (int i = 0; i < parts.length; i++) {
+                if (i != 0) {
+                    domNode.appendChild(page.createElement(HtmlBreak.TAG_NAME));
+                }
+                domNode.appendChild(new DomText(page, parts[i]));
+            }
         }
     }
 
@@ -758,7 +763,15 @@ protected void setInnerTextImpl(final String value) {
      */
     @Override
     public void setTextContent(final Object value) {
-        setInnerTextImpl(value == null ? null : Context.toString(value));
+        final DomNode domNode = getDomNodeOrDie();
+        domNode.removeAllChildren();
+
+        if (value != null) {
+            final String textValue = Context.toString(value);
+            if (StringUtils.isNotEmpty(textValue)) {
+                domNode.appendChild(new DomText(domNode.getPage(), textValue));
+            }
+        }
     }
 
     /**

diff --git a/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/html/HTMLHtmlElement.java b/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/html/HTMLHtmlElement.java
@@ -71,15 +71,6 @@ public void setOuterHTML(final Object value) {
         throw Context.reportRuntimeError("outerHTML is read-only for tag 'html'");
     }
 
-    /**
-     * Overwritten to throw an exception because this is readonly.
-     * @param value the new value for the contents of this node
-     */
-    @Override
-    protected void setInnerTextImpl(final String value) {
-        throw Context.reportRuntimeError("innerText is read-only for tag 'html'");
-    }
-
     /**
      * Returns {@code version} property.
      * @return the {@code version} property

diff --git a/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/html/HTMLElement2Test.java b/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/html/HTMLElement2Test.java
@@ -27,6 +27,7 @@
 
 import com.gargoylesoftware.htmlunit.BrowserRunner;
 import com.gargoylesoftware.htmlunit.BrowserRunner.Alerts;
+import com.gargoylesoftware.htmlunit.BrowserRunner.HtmlUnitNYI;
 import com.gargoylesoftware.htmlunit.BrowserRunner.NotYetImplemented;
 import com.gargoylesoftware.htmlunit.WebDriverTestCase;
 import com.gargoylesoftware.htmlunit.html.HtmlPageTest;
@@ -907,6 +908,50 @@ public void innerText_LineBreak() throws Exception {
         loadPageVerifyTitle2(html);
     }
 
+
+    /**
+     * @throws Exception if the test fails
+     */
+    @Test
+    @Alerts(DEFAULT = {"0", "1", " ", "0", "1", "undefined", "1", "[object Object]"},
+            IE = {"0", "1", "\u00A0", "1", "1", "undefined", "1", "[object Object]"})
+    @HtmlUnitNYI(IE = {"0", "1", " ", "1", "1", "undefined", "1", "[object Object]"})
+    public void innerText_Empty() throws Exception {
+        final String html = "<html><head>\n"
+            + "<script>\n"
+            + LOG_TITLE_FUNCTION
+            + "  function test() {\n"
+            + "    myTestDiv0.innerText = '';\n"
+            + "    log(myTestDiv0.childNodes.length);\n"
+
+            + "    myTestDiv1.innerText = ' ';\n"
+            + "    log(myTestDiv1.childNodes.length);\n"
+            + "    log(myTestDiv1.childNodes.item(0).data);\n"
+
+            + "    myTestDiv2.innerText = null;\n"
+            + "    log(myTestDiv2.childNodes.length);\n"
+
+            + "    myTestDiv3.innerText = undefined;\n"
+            + "    log(myTestDiv3.childNodes.length);\n"
+            + "    log(myTestDiv3.childNodes.item(0).data);\n"
+
+            + "    myTestDiv4.innerText = { a: 'b'};\n"
+            + "    log(myTestDiv4.childNodes.length);\n"
+            + "    log(myTestDiv4.childNodes.item(0).data);\n"
+            + "  }\n"
+            + "</script>\n"
+            + "</head>\n"
+            + "<body onload='test()'>\n"
+            + "  <div id='myTestDiv0'>something</div>\n"
+            + "  <div id='myTestDiv1'>something</div>\n"
+            + "  <div id='myTestDiv2'>something</div>\n"
+            + "  <div id='myTestDiv3'>something</div>\n"
+            + "  <div id='myTestDiv4'>something</div>\n"
+            + "</body></html>";
+
+        loadPageVerifyTitle2(html);
+    }
+
     /**
      * @throws Exception if the test fails
      */

diff --git a/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/html/HTMLElementTest.java b/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/html/HTMLElementTest.java
@@ -30,6 +30,7 @@
 
 import com.gargoylesoftware.htmlunit.BrowserRunner;
 import com.gargoylesoftware.htmlunit.BrowserRunner.Alerts;
+import com.gargoylesoftware.htmlunit.BrowserRunner.HtmlUnitNYI;
 import com.gargoylesoftware.htmlunit.BrowserRunner.NotYetImplemented;
 import com.gargoylesoftware.htmlunit.WebDriverTestCase;
 import com.gargoylesoftware.htmlunit.html.HtmlPageTest;
@@ -1875,7 +1876,11 @@ public void childrenFunctionAccess() throws Exception {
     @Test
     @Alerts(DEFAULT = {"Old = Old\n\ninnerText", "New = New cell value"},
             IE = {"Old = Old \ninnerText", "New = New cell value"})
-    @NotYetImplemented({CHROME, EDGE, FF, FF78})
+    @HtmlUnitNYI(CHROME = {"Old = Old\ninnerText", "New = New cell value"},
+            EDGE =  {"Old = Old\ninnerText", "New = New cell value"},
+            FF = {"Old = Old\ninnerText", "New = New cell value"},
+            FF78 = {"Old = Old\ninnerText", "New = New cell value"},
+            IE  = {"Old = Old\ninnerText", "New = New cell value"})
     public void getSetInnerTextSimple() throws Exception {
         final String html = "<html>\n"
             + "<head>\n"

diff --git a/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/html/HTMLHtmlElementTest.java b/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/html/HTMLHtmlElementTest.java
@@ -17,6 +17,7 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.openqa.selenium.By;
+import org.openqa.selenium.JavascriptExecutor;
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.WebElement;
 import org.openqa.selenium.htmlunit.HtmlUnitDriver;
@@ -32,6 +33,7 @@
  * @author Daniel Gredler
  * @author Marc Guillemot
  * @author Frank Danek
+ * @author Ronald Brill
  */
 @RunWith(BrowserRunner.class)
 public class HTMLHtmlElementTest extends WebDriverTestCase {
@@ -136,4 +138,30 @@ public void clientWidth() throws Exception {
 
         loadPageWithAlerts2(html);
     }
+
+    /**
+     * @throws Exception if the test fails
+     */
+    @Test
+    @Alerts("Hello World")
+    public void innerText() throws Exception {
+        final String html = "<html><head>\n"
+            + "<script>\n"
+            + LOG_TITLE_FUNCTION
+            + "  function test() {\n"
+            + "    document.documentElement.innerText = 'Hello World';\n"
+            + "  }\n"
+            + "</script>\n"
+            + "</head>\n"
+            + "<body onload='test()'>\n"
+            + "</body></html>";
+
+        final String js =
+                "if (document.documentElement.childNodes.length == 0) { return '0'; }"
+                + " return document.documentElement.childNodes.item(0).data;";
+
+        final WebDriver driver = loadPage2(html);
+        final String text = (String) ((JavascriptExecutor) driver).executeScript(js);
+        assertEquals(getExpectedAlerts()[0], text);
+    }
 }