[HtmlUnit] SVN: [15519] trunk/htmlunit/src

rbri--- via HtmlUnit-develop <[email protected]> Wed, 15 Aug 2018 08:01:27 +0000
Newsgroups gmane.comp.java.htmlunit.devel
Message-ID <[email protected]>
Revision: 15519
          http://sourceforge.net/p/htmlunit/code/15519
Author:   rbri
Date:     2018-08-15 08:01:20 +0000 (Wed, 15 Aug 2018)
Log Message:
-----------
next step in event refactoring (wip)

Modified Paths:
--------------
    trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/BrowserVersionFeatures.java
    trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
    trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/EventListenersContainer.java
    trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/EventTarget.java
    trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/Window3Test.java

Modified: trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/BrowserVersionFeatures.java
===================================================================
--- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/BrowserVersionFeatures.java	2018-08-15 07:14:08 UTC (rev 15518)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/BrowserVersionFeatures.java	2018-08-15 08:01:20 UTC (rev 15519)
@@ -899,10 +899,6 @@
     @BrowserFeature(FF)
     JS_EVENT_DISTINGUISH_PRINTABLE_KEY,
 
-    /** Executes the window event listeners if the node is detached from the document. */
-    @BrowserFeature(CHROME)
-    JS_EVENT_WINDOW_EXECUTE_IF_DITACHED,
-
     /** Whether {@code FileReader} includes content type or not. */
     @BrowserFeature(FF)
     JS_FILEREADER_CONTENT_TYPE,

Modified: trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
===================================================================
--- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java	2018-08-15 07:14:08 UTC (rev 15518)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java	2018-08-15 08:01:20 UTC (rev 15519)
@@ -141,6 +141,7 @@
  * @author Ronald Brill
  * @author Frank Danek
  * @author Joerg Werner
+ * @author Atsushi Nakagawa
  */
 public class HtmlPage extends SgmlPage {
 

Modified: trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/EventListenersContainer.java
===================================================================
--- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/EventListenersContainer.java	2018-08-15 07:14:08 UTC (rev 15518)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/EventListenersContainer.java	2018-08-15 08:01:20 UTC (rev 15519)
@@ -48,6 +48,7 @@
  * @author Ahmed Ashour
  * @author Frank Danek
  * @author Ronald Brill
+ * @author Atsushi Nakagawa
  */
 public class EventListenersContainer implements Serializable {
 

Modified: trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/EventTarget.java
===================================================================
--- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/EventTarget.java	2018-08-15 07:14:08 UTC (rev 15518)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/EventTarget.java	2018-08-15 08:01:20 UTC (rev 15519)
@@ -15,7 +15,6 @@
 package com.gargoylesoftware.htmlunit.javascript.host.event;
 
 import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.JS_CALL_RESULT_IS_LAST_RETURN_VALUE;
-import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.JS_EVENT_WINDOW_EXECUTE_IF_DITACHED;
 import static com.gargoylesoftware.htmlunit.javascript.configuration.SupportedBrowser.CHROME;
 import static com.gargoylesoftware.htmlunit.javascript.configuration.SupportedBrowser.EDGE;
 import static com.gargoylesoftware.htmlunit.javascript.configuration.SupportedBrowser.FF;
@@ -26,10 +25,8 @@
 import java.util.List;
 
 import org.apache.commons.lang3.StringUtils;
-import org.w3c.dom.Document;
 
 import com.gargoylesoftware.htmlunit.ScriptResult;
-import com.gargoylesoftware.htmlunit.html.DomDocumentFragment;
 import com.gargoylesoftware.htmlunit.html.DomElement;
 import com.gargoylesoftware.htmlunit.html.DomNode;
 import com.gargoylesoftware.htmlunit.html.HtmlElement;
@@ -39,7 +36,7 @@
 import com.gargoylesoftware.htmlunit.javascript.configuration.JsxConstructor;
 import com.gargoylesoftware.htmlunit.javascript.configuration.JsxFunction;
 import com.gargoylesoftware.htmlunit.javascript.host.Window;
-import com.gargoylesoftware.htmlunit.javascript.host.html.HTMLElement;
+import com.gargoylesoftware.htmlunit.javascript.host.dom.Document;
 
 import net.sourceforge.htmlunit.corejs.javascript.Context;
 import net.sourceforge.htmlunit.corejs.javascript.Function;
@@ -49,6 +46,8 @@
  * A JavaScript object for {@code EventTarget}.
  *
  * @author Ahmed Ashour
+ * @author Ronald Brill
+ * @author Atsushi Nakagawa
  */
 @JsxClass({CHROME, FF, EDGE})
 @JsxClass(isJSObject = false, value = IE)
@@ -122,45 +121,41 @@
         final Event previousEvent = window.getCurrentEvent();
         window.setCurrentEvent(event);
 
-        // The load event has some unnatural behaviour that we need to handle specially
-        final boolean isLoadEvent = Event.TYPE_LOAD.equals(event.getType());
-
         try {
             // These can be null if we aren't tied to a DOM node
             final DomNode ourNode = getDomNodeOrNull();
             final DomNode ourParentNode = (ourNode != null) ? ourNode.getParentNode() : null;
 
-            boolean isAttached = false;
-            for (DomNode node = ourNode; node != null; node = node.getParentNode()) {
-                if (node instanceof Document || node instanceof DomDocumentFragment) {
-                    isAttached = true;
-                    break;
-                }
-            }
-
             // Determine the propagation path which is fixed here and not affected by
             // DOM tree modification from intermediate listeners (tested in Chrome)
             final List<EventTarget> propagationPath = new ArrayList<>();
 
-            // The window 'load' event targets Document but paths Window only (tested in Chrome/FF)
-            if (!isLoadEvent || !(ourNode instanceof Document)) {
-                // We go on the propagation path first
-                if (isAttached || !(this instanceof HTMLElement)) {
-                    propagationPath.add(this);
+            // We're added to the propagation path first
+            propagationPath.add(this);
+
+            // Then add all our parents if we have any (pure JS object such as XMLHttpRequest
+            // and MessagePort, etc. will not have any parents)
+            for (DomNode parent = ourParentNode; parent != null; parent = parent.getParentNode()) {
+                propagationPath.add(parent.getScriptableObject());
+            }
+
+            // The load event has some unnatural behaviour that we need to handle specially
+            if (Event.TYPE_LOAD.equals(event.getType())) {
+
+                // The Window load event targets Document but paths Window only (tested in Chrome/FF)
+                if (this instanceof Document) {
+                    propagationPath.clear();
+                    propagationPath.add(window);
                 }
-                // Then add all our parents if we have any (pure JS object such as XMLHttpRequest
-                // and MessagePort, etc. will not have any parents)
-                for (DomNode parent = ourParentNode; parent != null; parent = parent.getParentNode()) {
-                    final EventTarget jsNode = parent.getScriptableObject();
-                    if (isAttached || !(jsNode instanceof HTMLElement)) {
-                        propagationPath.add(jsNode);
-                    }
+                else {
+                    // The load event for other elements target that element and but path only
+                    // up to Document and not Window, so do nothing here
+                    // (see Note in https://www.w3.org/TR/DOM-Level-3-Events/#event-type-load)
                 }
             }
-            // The 'load' event for other elements target that element and but does not path Window
-            // (see Note in https://www.w3.org/TR/DOM-Level-3-Events/#event-type-load)
-            if (!isLoadEvent || ourNode instanceof Document) {
-                if (isAttached || getBrowserVersion().hasFeature(JS_EVENT_WINDOW_EXECUTE_IF_DITACHED)) {
+            else {
+                // Add Window if the the propagation path reached Document
+                if (propagationPath.get(propagationPath.size() - 1) instanceof Document) {
                     propagationPath.add(window);
                 }
             }
@@ -167,22 +162,17 @@
 
             final boolean ie = getBrowserVersion().hasFeature(JS_CALL_RESULT_IS_LAST_RETURN_VALUE);
 
-            // Refactoring note: Not sure of the reasoning for this but preserving nonetheless: Nodes
-            // are traversed if they're attached or if they're non-HTMLElement.  However, the capturing
-            // phase only traverses nodes that are attached
-            if (isAttached) {
-                // capturing phase
-                event.setEventPhase(Event.CAPTURING_PHASE);
+            // capturing phase
+            event.setEventPhase(Event.CAPTURING_PHASE);
 
-                for (int i = propagationPath.size() - 1; i >= 1; i--) {
-                    final EventTarget jsNode = propagationPath.get(i);
-                    final EventListenersContainer elc = jsNode.eventListenersContainer_;
-                    if (elc != null) {
-                        final ScriptResult r = elc.executeCapturingListeners(event, args);
-                        result = ScriptResult.combine(r, result, ie);
-                        if (event.isPropagationStopped()) {
-                            return result;
-                        }
+            for (int i = propagationPath.size() - 1; i >= 1; i--) {
+                final EventTarget jsNode = propagationPath.get(i);
+                final EventListenersContainer elc = jsNode.eventListenersContainer_;
+                if (elc != null) {
+                    final ScriptResult r = elc.executeCapturingListeners(event, args);
+                    result = ScriptResult.combine(r, result, ie);
+                    if (event.isPropagationStopped()) {
+                        return result;
                     }
                 }
             }

Modified: trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/Window3Test.java
===================================================================
--- trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/Window3Test.java	2018-08-15 07:14:08 UTC (rev 15518)
+++ trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/Window3Test.java	2018-08-15 08:01:20 UTC (rev 15519)
@@ -52,6 +52,7 @@
  * @author Daniel Gredler
  * @author Frank Danek
  * @author Ronald Brill
+ * @author Atsushi Nakagawa
  */
 @RunWith(BrowserRunner.class)
 public class Window3Test extends WebDriverTestCase {
@@ -1889,7 +1890,6 @@
                         "document at load 2 capture",
                         "after"})
     public void onloadFrame() throws Exception {
-
         final String content = HtmlPageTest.STANDARDS_MODE_PREFIX_
             + "<html><head>\n"
             + "<script>\n"
@@ -1977,15 +1977,15 @@
      * @throws Exception if the test fails
      */
     @Test
-    @Alerts(DEFAULT = {"window at click 1 capture",
-                        "window at click 2 capture",
-                        "onclick 2",
-                        "i1 at click 1",
-                        "i1 at click 1 capture",
-                        "i1 at click 2",
-                        "i1 at click 2 capture",
-                        "window at click 1",
-                        "window at click 2"})
+    @Alerts({"window at click 1 capture",
+                "window at click 2 capture",
+                "onclick 2",
+                "i1 at click 1",
+                "i1 at click 1 capture",
+                "i1 at click 2",
+                "i1 at click 2 capture",
+                "window at click 1",
+                "window at click 2"})
     public void propagation() throws Exception {
         final String html = HtmlPageTest.STANDARDS_MODE_PREFIX_
             + "<html><head>\n"
@@ -2027,21 +2027,25 @@
      * @throws Exception if the test fails
      */
     @Test
-    @Alerts(DEFAULT = {"d1 at click 1 capture",
-                        "d1 at click 2 capture",
-                        "d2 at click 1 capture",
-                        "d2 at click 2 capture",
-                        "d3 at click 1",
-                        "d3 onclick",
-                        "d3 at click 1 capture",
-                        "d3 at click 2",
-                        "d3 at click 2 capture",
-                        "d2 at click 1",
-                        "d2 onclick",
-                        "d2 at click 2",
-                        "d1 at click 1",
-                        "d1 onclick",
-                        "d1 at click 2"})
+    @Alerts({"window at click 1 capture",
+                "window at click 2 capture",
+                "d1 at click 1 capture",
+                "d1 at click 2 capture",
+                "d2 at click 1 capture",
+                "d2 at click 2 capture",
+                "d3 at click 1",
+                "d3 onclick",
+                "d3 at click 1 capture",
+                "d3 at click 2",
+                "d3 at click 2 capture",
+                "d2 at click 1",
+                "d2 onclick",
+                "d2 at click 2",
+                "d1 at click 1",
+                "d1 onclick",
+                "d1 at click 2",
+                "window at click 1",
+                "window at click 2"})
     public void propagationNested() throws Exception {
         final String html = HtmlPageTest.STANDARDS_MODE_PREFIX_
             + "<html><head>\n"
@@ -2062,6 +2066,11 @@
             + "  <textarea id='log' rows=40 cols=80></textarea>\n"
 
             + "<script>\n"
+            + "  window.addEventListener('click', function () { log('window at click 1') })\n"
+            + "  window.addEventListener('click', function () { log('window at click 1 capture') }, true)\n"
+            + "  window.addEventListener('click', function () { log('window at click 2') })\n"
+            + "  window.addEventListener('click', function () { log('window at click 2 capture') }, true)\n"
+
             + "  d1.addEventListener('click', function () { log('d1 at click 1') })\n"
             + "  d1.onclick = function () { log('d1 onclick') }\n"
             + "  d1.addEventListener('click', function () { log('d1 at click 1 capture') }, true)\n"
@@ -2090,6 +2099,91 @@
     }
 
     /**
+     * Similar as {@link #propagationNested()} but clicking a detached element.
+     * Check bubbling propagation after modification of the DOM tree by an intermediate listener.
+     *
+     * @throws Exception if the test fails
+     */
+    @Test
+    @Alerts({"window at click 1 capture",
+                "window at click 2 capture",
+                "begin detach click",
+                "d2 at click 1 capture",
+                "d2 at click 2 capture",
+                "d3 at click 1",
+                "d3 onclick",
+                "d3 at click 1 capture",
+                "d3 at click 2",
+                "d3 at click 2 capture",
+                "d2 at click 1",
+                "d2 onclick",
+                "d2 at click 2",
+                "end detach click",
+                "window at click 1",
+                "window at click 2"})
+    public void propagationNestedDetached() throws Exception {
+        final String html = HtmlPageTest.STANDARDS_MODE_PREFIX_
+            + "<html><head>\n"
+            + "<script>\n"
+            + "  function log(msg) {\n"
+            + "    document.getElementById('log').value += msg + '\\n';\n"
+            + "  }\n"
+
+            + "  function detachAndClick() {\n"
+            + "    log('begin detach click')\n"
+            + "    var d2 = window.d2, d3 = window.d3\n"
+            + "    d2.parentNode.removeChild(d2);\n"
+            + "    d3.click();\n"
+            + "    log('end detach click')\n"
+            + "  }\n"
+            + "</script>\n"
+            + "</head>\n"
+            + "<body>\n"
+            + "  <div id='d1' style='width: 150px; height: 150px; background-color: blue'>\n"
+            + "    <div id='d2' style='width: 100px; height: 100px; background-color: green'>\n"
+            + "      <div id='d3' style='width: 50px; height: 50px; background-color: red'>\n"
+            + "      </div>\n"
+            + "    </div>\n"
+            + "  </div>\n"
+            + "  <input id='detach_click' type='button' value='Detach & click' onclick='detachAndClick()'>\n"
+
+            + "  <textarea id='log' rows=40 cols=80></textarea>\n"
+
+            + "<script>\n"
+            + "  d2 = window.d2, d3 = window.d3\n" // Save because "Detach & click" removes them
+            + "  window.addEventListener('click', function () { log('window at click 1') })\n"
+            + "  window.addEventListener('click', function () { log('window at click 1 capture') }, true)\n"
+            + "  window.addEventListener('click', function () { log('window at click 2') })\n"
+            + "  window.addEventListener('click', function () { log('window at click 2 capture') }, true)\n"
+
+            + "  d1.addEventListener('click', function () { log('d1 at click 1') })\n"
+            + "  d1.onclick = function () { log('d1 onclick') }\n"
+            + "  d1.addEventListener('click', function () { log('d1 at click 1 capture') }, true)\n"
+            + "  d1.addEventListener('click', function () { log('d1 at click 2') })\n"
+            + "  d1.addEventListener('click', function () { log('d1 at click 2 capture') }, true)\n"
+
+            + "  d2.addEventListener('click', function () { log('d2 at click 1') })\n"
+            + "  d2.onclick = function () { log('d2 onclick'); d2.parentNode.removeChild(d2) }\n"
+            + "  d2.addEventListener('click', function () { log('d2 at click 1 capture') }, true)\n"
+            + "  d2.addEventListener('click', function () { log('d2 at click 2') })\n"
+            + "  d2.addEventListener('click', function () { log('d2 at click 2 capture') }, true)\n"
+
+            + "  d3.addEventListener('click', function () { log('d3 at click 1') })\n"
+            + "  d3.onclick = function () { log('d3 onclick') }\n"
+            + "  d3.addEventListener('click', function () { log('d3 at click 1 capture') }, true)\n"
+            + "  d3.addEventListener('click', function () { log('d3 at click 2') })\n"
+            + "  d3.addEventListener('click', function () { log('d3 at click 2 capture') }, true)\n"
+            + "</script>\n"
+            + "</body></html>";
+
+        final WebDriver driver = loadPage2(html);
+        driver.findElement(By.id("detach_click")).click();
+
+        final String text = driver.findElement(By.id("log")).getAttribute("value").trim().replaceAll("\r", "");
+        assertEquals(String.join("\n", getExpectedAlerts()), text);
+    }
+
+    /**
      * This test determines that the return value of listeners are apparently
      * ignored and only that of the property handler is used.
      *
@@ -2096,11 +2190,11 @@
      * @throws Exception if the test fails
      */
     @Test
-    @Alerts(DEFAULT = {"listener: stop propagation & return false",
-                        "FIRED a1",
-                        "listener: return true",
-                        "property: return false",
-                        "listener: return true"})
+    @Alerts({"listener: stop propagation & return false",
+                "FIRED a1",
+                "listener: return true",
+                "property: return false",
+                "listener: return true"})
     public void stopPropagation() throws Exception {
         final String html = HtmlPageTest.STANDARDS_MODE_PREFIX_
             + "<html><head>\n"


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot