htmlunit-develop — General discussion of the development of HtmlUnit

[HtmlUnit] SVN: [15529] trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit

[<rb...@us...>]

Revision: 15529
          http://sourceforge.net/p/htmlunit/code/15529
Author:   rbri
Date:     2018-08-17 19:48:56 +0000 (Fri, 17 Aug 2018)
Log Message:
-----------
some cleanup

Modified Paths:
--------------
    trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
    trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/BeforeUnloadEvent.java
    trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/Event.java

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-17 19:28:08 UTC (rev 15528)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java	2018-08-17 19:48:56 UTC (rev 15529)
@@ -1288,8 +1288,8 @@
 
     private boolean isOnbeforeunloadAccepted(final HtmlPage page, final Event event, final ScriptResult result) {
         if (event instanceof BeforeUnloadEvent) {
-            if (((BeforeUnloadEvent) event).isBeforeUnloadMessageSet()) {
-                final String message = Context.toString(event.getReturnValue());
+            final BeforeUnloadEvent beforeUnloadEvent = (BeforeUnloadEvent) event;
+            if (beforeUnloadEvent.isBeforeUnloadMessageSet()) {
                 final OnbeforeunloadHandler handler = getWebClient().getOnbeforeunloadHandler();
                 if (handler == null) {
                     LOG.warn("document.onbeforeunload() returned a string in event.returnValue,"
@@ -1296,6 +1296,7 @@
                             + " but no onbeforeunload handler installed.");
                 }
                 else {
+                    final String message = Context.toString(beforeUnloadEvent.getReturnValue());
                     return handler.handleEvent(page, message);
                 }
             }

Modified: trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/BeforeUnloadEvent.java
===================================================================
--- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/BeforeUnloadEvent.java	2018-08-17 19:28:08 UTC (rev 15528)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/BeforeUnloadEvent.java	2018-08-17 19:48:56 UTC (rev 15529)
@@ -41,6 +41,7 @@
  */
 @JsxClass
 public class BeforeUnloadEvent extends Event {
+    private Object returnValue_;
 
     /**
      * Creates a new event instance.
@@ -84,11 +85,6 @@
         return Undefined.instance;
     }
 
-    @Override
-    protected boolean isReturnValueBackedByPreventDefault() {
-        return false;
-    }
-
     /**
      * @return {@code true} if returnValue holds the beforeunload message
      */
@@ -99,10 +95,9 @@
     /**
      * @return the return value associated with the event
      */
-    @Override
     @JsxGetter
     public Object getReturnValue() {
-        return super.getReturnValue();
+        return returnValue_;
     }
 
     /**
@@ -109,10 +104,9 @@
      * Sets the return value associated with the event.
      * @param returnValue the return value associated with the event
      */
-    @Override
     @JsxSetter
     public void setReturnValue(final Object returnValue) {
-        super.setReturnValue(returnValue);
+        returnValue_ = returnValue;
     }
 
     @Override

Modified: trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/Event.java
===================================================================
--- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/Event.java	2018-08-17 19:28:08 UTC (rev 15528)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/Event.java	2018-08-17 19:48:56 UTC (rev 15529)
@@ -16,7 +16,6 @@
 
 import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.EVENT_FOCUS_FOCUS_IN_BLUR_OUT;
 import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.EVENT_ONLOAD_CANCELABLE_FALSE;
-import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.EVENT_RETURN_VALUE_IS_PREVENT_DEFAULT;
 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;
@@ -36,7 +35,6 @@
 import com.gargoylesoftware.htmlunit.javascript.configuration.JsxSetter;
 
 import net.sourceforge.htmlunit.corejs.javascript.Context;
-import net.sourceforge.htmlunit.corejs.javascript.ScriptRuntime;
 import net.sourceforge.htmlunit.corejs.javascript.Scriptable;
 import net.sourceforge.htmlunit.corejs.javascript.ScriptableObject;
 import net.sourceforge.htmlunit.corejs.javascript.Undefined;
@@ -182,9 +180,8 @@
     private String propertyName_;
     private boolean stopPropagation_;
     private boolean stopImmediatePropagation_;
-    private Object returnValue_;
     private boolean preventDefault_;
-    private Boolean returnValueIsPreventDefault_;
+
     /**
      * The current event phase. This is a W3C standard attribute. One of {@link #NONE},
      * {@link #CAPTURING_PHASE}, {@link #AT_TARGET} or {@link #BUBBLING_PHASE}.
@@ -584,40 +581,6 @@
     }
 
     /**
-     * @return true if returnValue is backed by the same storage as preventDefault.
-     */
-    protected boolean isReturnValueBackedByPreventDefault() {
-        if (returnValueIsPreventDefault_ == null) {
-            returnValueIsPreventDefault_ = getBrowserVersion().hasFeature(EVENT_RETURN_VALUE_IS_PREVENT_DEFAULT);
-        }
-        return returnValueIsPreventDefault_;
-    }
-
-    /**
-     * Returns the return value associated with the event.
-     * @return the return value associated with the event
-     */
-    public Object getReturnValue() {
-        if (isReturnValueBackedByPreventDefault()) {
-            return !preventDefault_;
-        }
-        return returnValue_;
-    }
-
-    /**
-     * Sets the return value associated with the event.
-     * @param returnValue the return value associated with the event
-     */
-    public void setReturnValue(final Object returnValue) {
-        if (isReturnValueBackedByPreventDefault()) {
-            preventDefault_ = !ScriptRuntime.toBoolean(returnValue);
-        }
-        else {
-            returnValue_ = returnValue;
-        }
-    }
-
-    /**
      * Handles the return values of property handlers.
      * @param returnValue the return value returned by the property handler
      */

[HtmlUnit] [htmlunit:bugs] #1984 Reworking the JS Event listeners implementation

[RBRi <rb...@us...>]

Tests added and your code is hopefully merged in. But again more tests failing - have i done it wrong?


---

** [bugs:#1984] Reworking the JS Event listeners implementation**

**Status:** accepted
**Group:** Latest SVN
**Created:** Tue Aug 14, 2018 01:38 PM UTC by Atsushi Nakagawa
**Last Updated:** Fri Aug 17, 2018 11:52 AM UTC
**Owner:** RBRi
**Attachments:**

- [EventListenersContainer.java](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventListenersContainer.java) (16.9 kB; application/octet-stream)
- [EventListenersContainer.java-2.32.txt](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventListenersContainer.java-2.32.txt) (15.5 kB; text/plain)
- [EventTarget.java](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventTarget.java) (14.7 kB; application/octet-stream)
- [EventTarget.java-2.32.txt](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventTarget.java-2.32.txt) (13.4 kB; text/plain)


### Problem in brief

Thorough testing has revealed event listeners in HtmlUnit are implemented quite liberally in a few places with regards to how they should work.  It's somewhat surprising they're working so well in practice.

This bug report attempts to address the problems and tackle at fixing them.  Scope includes `xyz.addListenerEvent('foo', function () { ... })` listeners and `xyz.onfoo = function () { ... }` property handlers.

##### Problems noticed

* Ordering of listeners called during the *at target* phase should honour the order in which the listeners were added, but does not.
* Ordering of property handlers such as `window.onload` should honour the timing at which the property was set, with respect to other event listeners, but does not.
* Events where `event.bubbles` is `false` should not have a *bubbling* phase, but does.
* The load event for `Window`should only traverse `Window` but traverses `Window, Document` instead.
* The load event for everything else should traverse from `Document` to all levels down to the element in question, but traverses the element in question only.  (Every place using `EventTarget.executeEventLocally()` potentially applies to this and should probably be using the standard `EventTarget.fireEvent()` instead.)
* The load event for the `<frame>` element should be treated like  "everything else" above, but is not.  (On the other hand, the `onload` property for `<body>` and `<frameset>` is correctly tied to `Window`.)
* The propagation path of events should not be affected by changes to the DOM tree by intermediate listeners, and therefore the *bubbling* phase should always traverse the same nodes as the *capturing* phase only in reverse, but is and does not.
* The return value of event listeners should be ignored (really? it looks that way.. maybe it was different in older IE) and only that of the property handler should be used, but is not.

### Test cases

There are five tests which I'm putting in the comments because it's quite long.

* `test_onload.html`
* `test_frame.html`
* `test_click.html`
* `test_nested_click.html`
* `test_event_return_value.html`

These extra two I'm including but haven't fixed yet.  They're limited to the load/error behaviour of `<script>` and `<img>` and require fixing up some implementation in `HtmlScript` and `HtmlImage`.

* `test_script_onload.html`
* `test_img_onload.html`

### Possible fix

The fix spans three files:

* `html/HtmlPage.java`
* `javascript/host/event/EventListenersContainer.java`
* `javascript/host/event/EventTarget.java`

Changes to `HtmlPage` is the diff below. 

The changes for `EventListenersContainer.java` and `EventTarget.java` are rather extensive so I'm attaching the modified files along with their respective base versions suffixed with `-2.32.txt`.

```diff
--- a/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
+++ b/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
@@ -86,15 +86,16 @@
 import com.gargoylesoftware.htmlunit.javascript.PostponedAction;
 import com.gargoylesoftware.htmlunit.javascript.SimpleScriptable;
 import com.gargoylesoftware.htmlunit.javascript.host.Window;
-import com.gargoylesoftware.htmlunit.javascript.host.dom.Node;
 import com.gargoylesoftware.htmlunit.javascript.host.event.BeforeUnloadEvent;
 import com.gargoylesoftware.htmlunit.javascript.host.event.Event;
+import com.gargoylesoftware.htmlunit.javascript.host.event.EventTarget;
 import com.gargoylesoftware.htmlunit.javascript.host.html.HTMLDocument;
 import com.gargoylesoftware.htmlunit.protocol.javascript.JavaScriptURLConnection;
 import com.gargoylesoftware.htmlunit.util.EncodingSniffer;
 import com.gargoylesoftware.htmlunit.util.UrlUtils;
 
 import net.sourceforge.htmlunit.corejs.javascript.Context;
+import net.sourceforge.htmlunit.corejs.javascript.ContextFactory;
 import net.sourceforge.htmlunit.corejs.javascript.Function;
 import net.sourceforge.htmlunit.corejs.javascript.Script;
 import net.sourceforge.htmlunit.corejs.javascript.Scriptable;
@@ -1214,18 +1215,28 @@ private boolean executeEventHandlersIfNeeded(final String eventType) {
         // Execute the specified event on the document element.
         final WebWindow window = getEnclosingWindow();
         if (window.getScriptableObject() instanceof Window) {
-            final DomElement element = getDocumentElement();
-            if (element == null) { // happens for instance if document.documentElement has been removed from parent
-                return true;
-            }
+            // We need the 'Document' node for these load events but getDocumentElement() returns
+            // <html> (HtmlHtml) which is one below that.  The 'Document' node is incidentally just
+            // us (HtmlPage).  (Some tidbits at https://www.w3.org/TR/DOM-Level-3-Events/#event-flow)
+            final DomNode node = this;
             final Event event;
             if (eventType.equals(Event.TYPE_BEFORE_UNLOAD)) {
-                event = new BeforeUnloadEvent(element, eventType);
+                event = new BeforeUnloadEvent(node, eventType);
             }
             else {
-                event = new Event(element, eventType);
+                event = new Event(node, eventType);
+            }
+
+            // This is the same as DomElement.fireEvent() and was copied
+            // here so it could be used with HtmlPage.
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Firing " + event);
             }
-            final ScriptResult result = element.fireEvent(event);
+
+            final EventTarget jsNode = node.getScriptableObject();
+            final ContextFactory cf = ((JavaScriptEngine) getWebClient().getJavaScriptEngine()).getContextFactory();
+            final ScriptResult result = cf.call(cx -> jsNode.fireEvent(event));
+
             if (!isOnbeforeunloadAccepted(this, event, result)) {
                 return false;
             }
@@ -1253,7 +1264,12 @@ private boolean executeEventHandlersIfNeeded(final String eventType) {
                     else {
                         event = new Event(frame, eventType);
                     }
-                    final ScriptResult result = ((Node) frame.getScriptableObject()).executeEventLocally(event);
+
+                    // This fires the "load" event for the <frame> element which, like all non-window
+                    // load events, propagates up to Document but not Window.  The "load" event for
+                    // <frameset> on the other hand, like that of <body>, is handled above where it is
+                    // fired against Document and directed to Window.
+                    final ScriptResult result = frame.fireEvent(event);
                     if (!isOnbeforeunloadAccepted((HtmlPage) frame.getPage(), event, result)) {
                         return false;
                     }
```


---

Sent from sourceforge.net because htm...@li... is subscribed to https://sourceforge.net/p/htmlunit/bugs/

To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/htmlunit/admin/bugs/options.  Or, if this is a mailing list, you can unsubscribe from the mailing list.

[HtmlUnit] SVN: [15528] trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/ ScriptResult.java

[<rb...@us...>]

Revision: 15528
          http://sourceforge.net/p/htmlunit/code/15528
Author:   rbri
Date:     2018-08-17 19:28:08 +0000 (Fri, 17 Aug 2018)
Log Message:
-----------
code style

Modified Paths:
--------------
    trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/ScriptResult.java

Modified: trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/ScriptResult.java
===================================================================
--- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/ScriptResult.java	2018-08-17 19:25:25 UTC (rev 15527)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/ScriptResult.java	2018-08-17 19:28:08 UTC (rev 15528)
@@ -93,7 +93,6 @@
      * @return a composite {@link ScriptResult}, based on the two input {@link ScriptResult}s
      */
     public static ScriptResult combine(final ScriptResult newResult, final ScriptResult originalResult) {
-
         final Object jsResult;
         final Page page;
 

[HtmlUnit] SVN: [15527] trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/ libraries/Prototype171Test.java

[<rb...@us...>]

Revision: 15527
          http://sourceforge.net/p/htmlunit/code/15527
Author:   rbri
Date:     2018-08-17 19:25:25 +0000 (Fri, 17 Aug 2018)
Log Message:
-----------
next step in event refactoring (wip)

Modified Paths:
--------------
    trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/libraries/Prototype171Test.java

Modified: trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/libraries/Prototype171Test.java
===================================================================
--- trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/libraries/Prototype171Test.java	2018-08-17 19:21:47 UTC (rev 15526)
+++ trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/libraries/Prototype171Test.java	2018-08-17 19:25:25 UTC (rev 15527)
@@ -137,7 +137,6 @@
      * @throws Exception if test fails
      */
     @Test
-    @NotYetImplemented
     public void event() throws Exception {
         test("event_test.html");
     }

[HtmlUnit] SVN: [15526] trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/ javascript/host/event/Event2Test.java

[<rb...@us...>]

Revision: 15526
          http://sourceforge.net/p/htmlunit/code/15526
Author:   rbri
Date:     2018-08-17 19:21:47 +0000 (Fri, 17 Aug 2018)
Log Message:
-----------
next step in event refactoring (wip)

Modified Paths:
--------------
    trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/event/Event2Test.java

Modified: trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/event/Event2Test.java
===================================================================
--- trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/event/Event2Test.java	2018-08-17 18:17:29 UTC (rev 15525)
+++ trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/event/Event2Test.java	2018-08-17 19:21:47 UTC (rev 15526)
@@ -28,6 +28,7 @@
 import com.gargoylesoftware.htmlunit.BrowserRunner.Alerts;
 import com.gargoylesoftware.htmlunit.BrowserRunner.BuggyWebDriver;
 import com.gargoylesoftware.htmlunit.BrowserRunner.NotYetImplemented;
+import com.gargoylesoftware.htmlunit.html.HtmlPageTest;
 import com.gargoylesoftware.htmlunit.WebDriverTestCase;
 
 /**
@@ -874,4 +875,87 @@
 
         loadPageWithAlerts2(html);
     }
+
+    /**
+     * @throws Exception if an error occurs
+     */
+    @Test
+    @Alerts({"anchor onclick prevented=false",
+                "document onclick prevented=false",
+                "window onclick prevented=true"})
+    public void returnPriority() throws Exception {
+        final String html = HtmlPageTest.STANDARDS_MODE_PREFIX_
+                + "<html><head>\n"
+                + "<script>\n"
+                + "  function log(msg) {\n"
+                + "    window.document.title += msg + ';';\n"
+                + "  }\n"
+                + "</script>\n"
+                + "</head>\n"
+                + "<body>\n"
+                + "<a id='tester' href='javascript:log(\"FIRED\")'>test: onclick return value</a>\n"
+                + "<script>\n"
+                + "  tester.onclick = function (event) { "
+                                    + "log('anchor onclick prevented=' + event.defaultPrevented); return true }\n"
+                + "  document.onclick = function (event) { "
+                                    + "log('document onclick prevented=' + event.defaultPrevented); return false }\n"
+                + "  window.onclick = function (event) { "
+                                    + "log('window onclick prevented=' + event.defaultPrevented); return true; }\n"
+                + "</script>\n"
+                + "</body></html>";
+
+        final WebDriver driver = loadPage2(html);
+        driver.findElement(By.id("tester")).click();
+
+        final String text = driver.getTitle().trim().replaceAll(";", "\n").trim();
+        assertEquals(String.join("\n", getExpectedAlerts()), text);
+    }
+
+    /**
+     * BeforeUnloadEvent's returnValue behaves differently to other events.
+     * @throws Exception if an error occurs
+     */
+    @Test
+    @Alerts(DEFAULT = {"nullwindow at beforeunload rv=",
+                        "window onbeforeunload rv=1",
+                        "window at beforeunload rv=1"},
+            IE = {"nullwindow at beforeunload rv=undefined",
+                        "window onbeforeunload rv=1",
+                        "window at beforeunload rv=2"})
+    public void returnPriority2() throws Exception {
+        final String html = HtmlPageTest.STANDARDS_MODE_PREFIX_
+                + "<html><head>\n"
+                + "<script>\n"
+                + "  output = '';\n"
+                + "  function log(msg) {\n"
+                + "    var output = localStorage.getItem('output');\n"
+                + "    output += msg + ';';\n"
+                + "    localStorage.setItem('output', output);\n"
+                + "  }\n"
+                + "</script>\n"
+                + "</head>\n"
+                + "<body>\n"
+                + "<button id='tester' onclick='window.location.reload()'>test: onbeforeunload return value</button>\n"
+
+                + "<button id='getResult' "
+                        + "onclick='window.document.title=localStorage.getItem(\"output\")'>get result</button>\n"
+                + "<script>\n"
+                + "  window.addEventListener('beforeunload', function (event) { "
+                        + "log('window at beforeunload rv=' + event.returnValue); event.returnValue='1' });\n"
+                + "  window.onbeforeunload = function (event) { "
+                        + "log('window onbeforeunload rv=' + event.returnValue); return '2' }\n"
+                + "  window.addEventListener('beforeunload', function (event) { "
+                        + "log('window at beforeunload rv=' + event.returnValue); event.returnValue='3' });\n"
+                + "</script>\n"
+                + "</body></html>";
+
+        final WebDriver driver = loadPage2(html);
+        driver.findElement(By.id("tester")).click();
+        driver.switchTo().alert().accept();
+        Thread.sleep(200); // avoid stale element exception in FF60
+
+        driver.findElement(By.id("getResult")).click();
+        final String text = driver.getTitle().trim().replaceAll(";", "\n").trim();
+        assertEquals(String.join("\n", getExpectedAlerts()), text);
+    }
 }

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

[<rb...@us...>]

Revision: 15525
          http://sourceforge.net/p/htmlunit/code/15525
Author:   rbri
Date:     2018-08-17 18:17:29 +0000 (Fri, 17 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/ScriptResult.java
    trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
    trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/BeforeUnloadEvent.java
    trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/Event.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/event/BeforeUnloadEvent2Test.java
    trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/event/Event3Test.java

Modified: trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/BrowserVersionFeatures.java
===================================================================
--- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/BrowserVersionFeatures.java	2018-08-16 14:32:55 UTC (rev 15524)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/BrowserVersionFeatures.java	2018-08-17 18:17:29 UTC (rev 15525)
@@ -154,9 +154,9 @@
     @BrowserFeature(IE)
     DOM_NORMALIZE_REMOVE_CHILDREN,
 
-    /** Event false result. */
-    @BrowserFeature(IE)
-    EVENT_FALSE_RESULT,
+    /** Indicates handler return value is only used if returnValue is the default value. */
+    @BrowserFeature({CHROME, FF})
+    EVENT_BEFORE_UNLOAD_USES_HANDLER_RETURN_ONLY_IF_FIRST,
 
     /** Triggers the onfocus onfocusin blur onfocusout events in this order. */
     @BrowserFeature(CHROME)
@@ -234,6 +234,10 @@
     @BrowserFeature(FF60)
     EVENT_ONPOPSTATE_DOCUMENT_CREATE_NOT_SUPPORTED,
 
+    /** Indicates if event.returnValue is backed by !event.defaultPrevented. */
+    @BrowserFeature({CHROME, EDGE})
+    EVENT_RETURN_VALUE_IS_PREVENT_DEFAULT,
+
     /** Supports event type 'BeforeUnloadEvent'. */
     @BrowserFeature({CHROME, FF})
     EVENT_TYPE_BEFOREUNLOADEVENT,
@@ -638,10 +642,6 @@
     @BrowserFeature(IE)
     JS_BOUNDINGCLIENTRECT_THROWS_IF_DISCONNECTED,
 
-    /** If we're emulating IE, the overall JavaScript return value is the last return value. */
-    @BrowserFeature(IE)
-    JS_CALL_RESULT_IS_LAST_RETURN_VALUE,
-
     /** toDataURL for canvas returns the CHROME version of the PNG. */
     @BrowserFeature(CHROME)
     JS_CANVAS_DATA_URL_CHROME_PNG,

Modified: trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/ScriptResult.java
===================================================================
--- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/ScriptResult.java	2018-08-16 14:32:55 UTC (rev 15524)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/ScriptResult.java	2018-08-17 18:17:29 UTC (rev 15525)
@@ -90,45 +90,24 @@
      *
      * @param newResult the new {@link ScriptResult} (may be {@code null})
      * @param originalResult the original {@link ScriptResult} (may be {@code null})
-     * @param ie whether or not we are emulating IE
      * @return a composite {@link ScriptResult}, based on the two input {@link ScriptResult}s
      */
-    public static ScriptResult combine(final ScriptResult newResult, final ScriptResult originalResult,
-        final boolean ie) {
+    public static ScriptResult combine(final ScriptResult newResult, final ScriptResult originalResult) {
 
         final Object jsResult;
         final Page page;
 
-        // If we're emulating IE, the overall JavaScript return value is the last return value.
-        // If we're emulating FF, the overall JavaScript return value is false if the return value
-        // was false at any level.
-        if (ie) {
-            if (newResult != null && !ScriptResult.isUndefined(newResult)) {
-                jsResult = newResult.getJavaScriptResult();
-            }
-            else if (originalResult != null) {
-                jsResult = originalResult.getJavaScriptResult();
-            }
-            else if (newResult != null) {
-                jsResult = newResult.getJavaScriptResult();
-            }
-            else {
-                jsResult = null;
-            }
+        if (ScriptResult.isFalse(newResult)) {
+            jsResult = newResult.getJavaScriptResult();
         }
+        else if (originalResult != null) {
+            jsResult = originalResult.getJavaScriptResult();
+        }
+        else if (newResult != null) {
+            jsResult = newResult.getJavaScriptResult();
+        }
         else {
-            if (ScriptResult.isFalse(newResult)) {
-                jsResult = newResult.getJavaScriptResult();
-            }
-            else if (originalResult != null) {
-                jsResult = originalResult.getJavaScriptResult();
-            }
-            else if (newResult != null) {
-                jsResult = newResult.getJavaScriptResult();
-            }
-            else {
-                jsResult = null;
-            }
+            jsResult = null;
         }
 
         // The new page is always the newest page.

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-16 14:32:55 UTC (rev 15524)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java	2018-08-17 18:17:29 UTC (rev 15525)
@@ -17,7 +17,6 @@
 import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.EVENT_FOCUS_FOCUS_IN_BLUR_OUT;
 import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.EVENT_FOCUS_IN_FOCUS_OUT_BLUR;
 import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.FOCUS_BODY_ELEMENT_AT_START;
-import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.JS_CALL_RESULT_IS_LAST_RETURN_VALUE;
 import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.JS_DEFERRED;
 import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.JS_IGNORES_UTF8_BOM_SOMETIMES;
 import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.PAGE_SELECTION_RANGE_FROM_SELECTABLE_TEXT_INPUT;
@@ -99,7 +98,6 @@
 import net.sourceforge.htmlunit.corejs.javascript.Script;
 import net.sourceforge.htmlunit.corejs.javascript.Scriptable;
 import net.sourceforge.htmlunit.corejs.javascript.ScriptableObject;
-import net.sourceforge.htmlunit.corejs.javascript.Undefined;
 
 /**
  * A representation of an HTML page returned from a server.
@@ -1289,10 +1287,9 @@
     }
 
     private boolean isOnbeforeunloadAccepted(final HtmlPage page, final Event event, final ScriptResult result) {
-        if (event.getType().equals(Event.TYPE_BEFORE_UNLOAD)) {
-            final boolean ie = hasFeature(JS_CALL_RESULT_IS_LAST_RETURN_VALUE);
-            final String message = getBeforeUnloadMessage(event, result, ie);
-            if (message != null) {
+        if (event instanceof BeforeUnloadEvent) {
+            if (((BeforeUnloadEvent) event).isBeforeUnloadMessageSet()) {
+                final String message = Context.toString(event.getReturnValue());
                 final OnbeforeunloadHandler handler = getWebClient().getOnbeforeunloadHandler();
                 if (handler == null) {
                     LOG.warn("document.onbeforeunload() returned a string in event.returnValue,"
@@ -1306,30 +1303,6 @@
         return true;
     }
 
-    private static String getBeforeUnloadMessage(final Event event, final ScriptResult result, final boolean ie) {
-        String message = null;
-        if (event.getReturnValue() != Undefined.instance) {
-            if (!ie || event.getReturnValue() != null || result == null || result.getJavaScriptResult() == null
-                    || result.getJavaScriptResult() == Undefined.instance) {
-                message = Context.toString(event.getReturnValue());
-            }
-        }
-        else {
-            if (result != null) {
-                if (ie) {
-                    if (result.getJavaScriptResult() != Undefined.instance) {
-                        message = Context.toString(result.getJavaScriptResult());
-                    }
-                }
-                else if (result.getJavaScriptResult() != null
-                        && result.getJavaScriptResult() != Undefined.instance) {
-                    message = Context.toString(result.getJavaScriptResult());
-                }
-            }
-        }
-        return message;
-    }
-
     /**
      * If a refresh has been specified either through a meta tag or an HTTP
      * response header, then perform that refresh.

Modified: trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/BeforeUnloadEvent.java
===================================================================
--- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/BeforeUnloadEvent.java	2018-08-16 14:32:55 UTC (rev 15524)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/BeforeUnloadEvent.java	2018-08-17 18:17:29 UTC (rev 15525)
@@ -14,10 +14,12 @@
  */
 package com.gargoylesoftware.htmlunit.javascript.host.event;
 
+import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.EVENT_BEFORE_UNLOAD_USES_HANDLER_RETURN_ONLY_IF_FIRST;
 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;
 
+import com.gargoylesoftware.htmlunit.BrowserVersion;
 import com.gargoylesoftware.htmlunit.html.DomNode;
 import com.gargoylesoftware.htmlunit.javascript.configuration.JsxClass;
 import com.gargoylesoftware.htmlunit.javascript.configuration.JsxConstructor;
@@ -34,6 +36,8 @@
  *
  * @author Frank Danek
  * @author Ahmed Ashour
+ * @author Ronald Brill
+ * @author Atsushi Nakagawa
  */
 @JsxClass
 public class BeforeUnloadEvent extends Event {
@@ -64,11 +68,35 @@
         super(domNode, type);
 
         setBubbles(false);
-        setReturnValue(Undefined.instance);
+        setReturnValue(getReturnValueDefault(getBrowserVersion()));
     }
 
+    @Override
+    public void initEvent(final String type, final boolean bubbles, final boolean cancelable) {
+        super.initEvent(type, bubbles, cancelable);
+        setReturnValue(getReturnValueDefault(getBrowserVersion()));
+    }
+
+    private static Object getReturnValueDefault(final BrowserVersion browserVersion) {
+        if (browserVersion.isChrome() || browserVersion.isFirefox()) {
+            return "";
+        }
+        return Undefined.instance;
+    }
+
+    @Override
+    protected boolean isReturnValueBackedByPreventDefault() {
+        return false;
+    }
+
     /**
-     * Returns the return value associated with the event.
+     * @return {@code true} if returnValue holds the beforeunload message
+     */
+    public boolean isBeforeUnloadMessageSet() {
+        return !getReturnValueDefault(getBrowserVersion()).equals(getReturnValue());
+    }
+
+    /**
      * @return the return value associated with the event
      */
     @Override
@@ -86,4 +114,18 @@
     public void setReturnValue(final Object returnValue) {
         super.setReturnValue(returnValue);
     }
+
+    @Override
+    void handlePropertyHandlerReturnValue(final Object returnValue) {
+        super.handlePropertyHandlerReturnValue(returnValue);
+
+        final BrowserVersion browserVersion = getBrowserVersion();
+
+        if (!Undefined.isUndefined(returnValue) && (returnValue != null || browserVersion.isIE())) {
+            if (!browserVersion.hasFeature(EVENT_BEFORE_UNLOAD_USES_HANDLER_RETURN_ONLY_IF_FIRST)
+                    || !getReturnValueDefault(browserVersion).equals(getReturnValue())) {
+                setReturnValue(returnValue);
+            }
+        }
+    }
 }

Modified: trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/Event.java
===================================================================
--- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/Event.java	2018-08-16 14:32:55 UTC (rev 15524)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/Event.java	2018-08-17 18:17:29 UTC (rev 15525)
@@ -16,6 +16,7 @@
 
 import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.EVENT_FOCUS_FOCUS_IN_BLUR_OUT;
 import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.EVENT_ONLOAD_CANCELABLE_FALSE;
+import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.EVENT_RETURN_VALUE_IS_PREVENT_DEFAULT;
 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;
@@ -35,6 +36,7 @@
 import com.gargoylesoftware.htmlunit.javascript.configuration.JsxSetter;
 
 import net.sourceforge.htmlunit.corejs.javascript.Context;
+import net.sourceforge.htmlunit.corejs.javascript.ScriptRuntime;
 import net.sourceforge.htmlunit.corejs.javascript.Scriptable;
 import net.sourceforge.htmlunit.corejs.javascript.ScriptableObject;
 import net.sourceforge.htmlunit.corejs.javascript.Undefined;
@@ -57,6 +59,7 @@
  * @author Rob Di Marco
  * @author Ronald Brill
  * @author Frank Danek
+ * @author Atsushi Nakagawa
  */
 @JsxClass
 public class Event extends SimpleScriptable {
@@ -181,7 +184,7 @@
     private boolean stopImmediatePropagation_;
     private Object returnValue_;
     private boolean preventDefault_;
-
+    private Boolean returnValueIsPreventDefault_;
     /**
      * The current event phase. This is a W3C standard attribute. One of {@link #NONE},
      * {@link #CAPTURING_PHASE}, {@link #AT_TARGET} or {@link #BUBBLING_PHASE}.
@@ -581,10 +584,23 @@
     }
 
     /**
+     * @return true if returnValue is backed by the same storage as preventDefault.
+     */
+    protected boolean isReturnValueBackedByPreventDefault() {
+        if (returnValueIsPreventDefault_ == null) {
+            returnValueIsPreventDefault_ = getBrowserVersion().hasFeature(EVENT_RETURN_VALUE_IS_PREVENT_DEFAULT);
+        }
+        return returnValueIsPreventDefault_;
+    }
+
+    /**
      * Returns the return value associated with the event.
      * @return the return value associated with the event
      */
     public Object getReturnValue() {
+        if (isReturnValueBackedByPreventDefault()) {
+            return !preventDefault_;
+        }
         return returnValue_;
     }
 
@@ -593,10 +609,25 @@
      * @param returnValue the return value associated with the event
      */
     public void setReturnValue(final Object returnValue) {
-        returnValue_ = returnValue;
+        if (isReturnValueBackedByPreventDefault()) {
+            preventDefault_ = !ScriptRuntime.toBoolean(returnValue);
+        }
+        else {
+            returnValue_ = returnValue;
+        }
     }
 
     /**
+     * Handles the return values of property handlers.
+     * @param returnValue the return value returned by the property handler
+     */
+    void handlePropertyHandlerReturnValue(final Object returnValue) {
+        if (Boolean.FALSE.equals(returnValue)) {
+            preventDefault();
+        }
+    }
+
+    /**
      * Returns the property name associated with the event.
      * @return the property name associated with the event
      */
@@ -621,9 +652,6 @@
                 final Method readMethod = klass.getMethod("getReturnValue");
                 final Method writeMethod = klass.getMethod("setReturnValue", Object.class);
                 defineProperty("returnValue", null, readMethod, writeMethod, ScriptableObject.EMPTY);
-                if ("Event".equals(klass.getSimpleName())) {
-                    setReturnValue(Boolean.TRUE);
-                }
             }
             catch (final Exception e) {
                 throw Context.throwAsScriptRuntimeEx(e);

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-16 14:32:55 UTC (rev 15524)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/EventListenersContainer.java	2018-08-17 18:17:29 UTC (rev 15525)
@@ -14,8 +14,6 @@
  */
 package com.gargoylesoftware.htmlunit.javascript.host.event;
 
-import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.EVENT_FALSE_RESULT;
-
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -345,21 +343,14 @@
                 if (function != null) {
                     final ScriptResult result =
                             page.executeJavaScriptFunction(function, thisObject, args, node);
-                    // Return value is only honoured for property handlers (Chrome/FF)
+                    // Return value is only honored for property handlers (Tested in Chrome/FF/IE11)
                     if (isPropertyHandler) {
                         allResult = result;
+                        event.handlePropertyHandlerReturnValue(result.getJavaScriptResult());
+
+                        // This return value is now all but unused and can be refactored away
+                        allResult = null;
                     }
-                    if (jsNode_.getBrowserVersion().hasFeature(EVENT_FALSE_RESULT)) {
-                        if (ScriptResult.isFalse(result)) {
-                            allResult = result;
-                        }
-                        else {
-                            final Object eventReturnValue = event.getReturnValue();
-                            if (eventReturnValue instanceof Boolean && !((Boolean) eventReturnValue).booleanValue()) {
-                                allResult = new ScriptResult(Boolean.FALSE, page);
-                            }
-                        }
-                    }
                 }
                 if (event.isImmediatePropagationStopped()) {
                     return allResult;

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-16 14:32:55 UTC (rev 15524)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/EventTarget.java	2018-08-17 18:17:29 UTC (rev 15525)
@@ -14,7 +14,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.javascript.configuration.SupportedBrowser.CHROME;
 import static com.gargoylesoftware.htmlunit.javascript.configuration.SupportedBrowser.EDGE;
 import static com.gargoylesoftware.htmlunit.javascript.configuration.SupportedBrowser.FF;
@@ -152,8 +151,6 @@
                 }
             }
 
-            final boolean ie = getBrowserVersion().hasFeature(JS_CALL_RESULT_IS_LAST_RETURN_VALUE);
-
             // capturing phase
             event.setEventPhase(Event.CAPTURING_PHASE);
 
@@ -162,7 +159,7 @@
                 final EventListenersContainer elc = jsNode.eventListenersContainer_;
                 if (elc != null) {
                     final ScriptResult r = elc.executeCapturingListeners(event, args);
-                    result = ScriptResult.combine(r, result, ie);
+                    result = ScriptResult.combine(r, result);
                     if (event.isPropagationStopped()) {
                         return result;
                     }
@@ -179,7 +176,7 @@
                 final EventListenersContainer elc = jsNode.eventListenersContainer_;
                 if (elc != null) {
                     final ScriptResult r = elc.executeAtTargetListeners(event, args);
-                    result = ScriptResult.combine(r, result, ie);
+                    result = ScriptResult.combine(r, result);
                     if (event.isPropagationStopped()) {
                         return result;
                     }
@@ -208,7 +205,7 @@
                     final EventListenersContainer elc = jsNode.eventListenersContainer_;
                     if (elc != null) {
                         final ScriptResult r = elc.executeBubblingListeners(event, args);
-                        result = ScriptResult.combine(r, result, ie);
+                        result = ScriptResult.combine(r, result);
                         if (event.isPropagationStopped()) {
                             return result;
                         }

Modified: trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/event/BeforeUnloadEvent2Test.java
===================================================================
--- trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/event/BeforeUnloadEvent2Test.java	2018-08-16 14:32:55 UTC (rev 15524)
+++ trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/event/BeforeUnloadEvent2Test.java	2018-08-17 18:17:29 UTC (rev 15525)
@@ -152,5 +152,4 @@
         onbeforeunload("e.returnValue = 'Hello';\n"
                 + "return 'Hello'");
     }
-
 }

Modified: trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/event/Event3Test.java
===================================================================
--- trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/event/Event3Test.java	2018-08-16 14:32:55 UTC (rev 15524)
+++ trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/event/Event3Test.java	2018-08-17 18:17:29 UTC (rev 15525)
@@ -14,8 +14,6 @@
  */
 package com.gargoylesoftware.htmlunit.javascript.host.event;
 
-import static com.gargoylesoftware.htmlunit.BrowserRunner.TestedBrowser.IE;
-
 import java.util.ArrayList;
 import java.util.List;
 
@@ -221,7 +219,6 @@
      */
     @Test
     @Alerts("false") // here not alerts! ;-)
-    @NotYetImplemented(IE)
     public void eventBubblingReturns_2() throws Exception {
         final boolean changesPage = Boolean.parseBoolean(getExpectedAlerts()[0]);
         testEventBubblingReturns("return true; ", "return false;", "return true; ", changesPage);

[HtmlUnit] SVN: [15524] trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit

[<rb...@us...>]

Revision: 15524
          http://sourceforge.net/p/htmlunit/code/15524
Author:   rbri
Date:     2018-08-16 14:32:55 +0000 (Thu, 16 Aug 2018)
Log Message:
-----------
next step in event refactoring (wip)

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

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-16 06:41:09 UTC (rev 15523)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java	2018-08-16 14:32:55 UTC (rev 15524)
@@ -1223,7 +1223,16 @@
             if (LOG.isDebugEnabled()) {
                 LOG.debug("Firing " + event);
             }
-            final EventTarget jsNode = this.getScriptableObject();
+
+            final EventTarget jsNode;
+            if (Event.TYPE_DOM_DOCUMENT_LOADED.equals(eventType)) {
+                jsNode = this.getScriptableObject();
+            }
+            else {
+                // The load/beforeunload/unload events target Document but paths Window only (tested in Chrome/FF)
+                jsNode = window.getScriptableObject();
+            }
+
             final ContextFactory cf = ((JavaScriptEngine) getWebClient().getJavaScriptEngine()).getContextFactory();
             final ScriptResult result = cf.call(cx -> jsNode.fireEvent(event));
 

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-16 06:41:09 UTC (rev 15523)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/EventTarget.java	2018-08-16 14:32:55 UTC (rev 15524)
@@ -141,17 +141,9 @@
 
             // 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);
-                }
-                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 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)
             }
             else {
                 // Add Window if the the propagation path reached Document

[HtmlUnit] [htmlunit:bugs] #1984 Reworking the JS Event listeners implementation

[RBRi <rb...@us...>]

Why you think we should change this?

-    @Alerts(DEFAULT = "frame1",
-            CHROME = {})
+    @Alerts(DEFAULT = "frame1")
     public void thisInEventHandler() throws Exception {
     
     When running these test with real browsers i got excactly the original results. Do you get really "frame1" with Chrome?


---

** [bugs:#1984] Reworking the JS Event listeners implementation**

**Status:** accepted
**Group:** Latest SVN
**Created:** Tue Aug 14, 2018 01:38 PM UTC by Atsushi Nakagawa
**Last Updated:** Thu Aug 16, 2018 06:59 AM UTC
**Owner:** RBRi
**Attachments:**

- [EventListenersContainer.java](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventListenersContainer.java) (16.9 kB; application/octet-stream)
- [EventListenersContainer.java-2.32.txt](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventListenersContainer.java-2.32.txt) (15.5 kB; text/plain)
- [EventTarget.java](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventTarget.java) (14.7 kB; application/octet-stream)
- [EventTarget.java-2.32.txt](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventTarget.java-2.32.txt) (13.4 kB; text/plain)


### Problem in brief

Thorough testing has revealed event listeners in HtmlUnit are implemented quite liberally in a few places with regards to how they should work.  It's somewhat surprising they're working so well in practice.

This bug report attempts to address the problems and tackle at fixing them.  Scope includes `xyz.addListenerEvent('foo', function () { ... })` listeners and `xyz.onfoo = function () { ... }` property handlers.

##### Problems noticed

* Ordering of listeners called during the *at target* phase should honour the order in which the listeners were added, but does not.
* Ordering of property handlers such as `window.onload` should honour the timing at which the property was set, with respect to other event listeners, but does not.
* Events where `event.bubbles` is `false` should not have a *bubbling* phase, but does.
* The load event for `Window`should only traverse `Window` but traverses `Window, Document` instead.
* The load event for everything else should traverse from `Document` to all levels down to the element in question, but traverses the element in question only.  (Every place using `EventTarget.executeEventLocally()` potentially applies to this and should probably be using the standard `EventTarget.fireEvent()` instead.)
* The load event for the `<frame>` element should be treated like  "everything else" above, but is not.  (On the other hand, the `onload` property for `<body>` and `<frameset>` is correctly tied to `Window`.)
* The propagation path of events should not be affected by changes to the DOM tree by intermediate listeners, and therefore the *bubbling* phase should always traverse the same nodes as the *capturing* phase only in reverse, but is and does not.
* The return value of event listeners should be ignored (really? it looks that way.. maybe it was different in older IE) and only that of the property handler should be used, but is not.

### Test cases

There are five tests which I'm putting in the comments because it's quite long.

* `test_onload.html`
* `test_frame.html`
* `test_click.html`
* `test_nested_click.html`
* `test_event_return_value.html`

These extra two I'm including but haven't fixed yet.  They're limited to the load/error behaviour of `<script>` and `<img>` and require fixing up some implementation in `HtmlScript` and `HtmlImage`.

* `test_script_onload.html`
* `test_img_onload.html`

### Possible fix

The fix spans three files:

* `html/HtmlPage.java`
* `javascript/host/event/EventListenersContainer.java`
* `javascript/host/event/EventTarget.java`

Changes to `HtmlPage` is the diff below. 

The changes for `EventListenersContainer.java` and `EventTarget.java` are rather extensive so I'm attaching the modified files along with their respective base versions suffixed with `-2.32.txt`.

```diff
--- a/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
+++ b/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
@@ -86,15 +86,16 @@
 import com.gargoylesoftware.htmlunit.javascript.PostponedAction;
 import com.gargoylesoftware.htmlunit.javascript.SimpleScriptable;
 import com.gargoylesoftware.htmlunit.javascript.host.Window;
-import com.gargoylesoftware.htmlunit.javascript.host.dom.Node;
 import com.gargoylesoftware.htmlunit.javascript.host.event.BeforeUnloadEvent;
 import com.gargoylesoftware.htmlunit.javascript.host.event.Event;
+import com.gargoylesoftware.htmlunit.javascript.host.event.EventTarget;
 import com.gargoylesoftware.htmlunit.javascript.host.html.HTMLDocument;
 import com.gargoylesoftware.htmlunit.protocol.javascript.JavaScriptURLConnection;
 import com.gargoylesoftware.htmlunit.util.EncodingSniffer;
 import com.gargoylesoftware.htmlunit.util.UrlUtils;
 
 import net.sourceforge.htmlunit.corejs.javascript.Context;
+import net.sourceforge.htmlunit.corejs.javascript.ContextFactory;
 import net.sourceforge.htmlunit.corejs.javascript.Function;
 import net.sourceforge.htmlunit.corejs.javascript.Script;
 import net.sourceforge.htmlunit.corejs.javascript.Scriptable;
@@ -1214,18 +1215,28 @@ private boolean executeEventHandlersIfNeeded(final String eventType) {
         // Execute the specified event on the document element.
         final WebWindow window = getEnclosingWindow();
         if (window.getScriptableObject() instanceof Window) {
-            final DomElement element = getDocumentElement();
-            if (element == null) { // happens for instance if document.documentElement has been removed from parent
-                return true;
-            }
+            // We need the 'Document' node for these load events but getDocumentElement() returns
+            // <html> (HtmlHtml) which is one below that.  The 'Document' node is incidentally just
+            // us (HtmlPage).  (Some tidbits at https://www.w3.org/TR/DOM-Level-3-Events/#event-flow)
+            final DomNode node = this;
             final Event event;
             if (eventType.equals(Event.TYPE_BEFORE_UNLOAD)) {
-                event = new BeforeUnloadEvent(element, eventType);
+                event = new BeforeUnloadEvent(node, eventType);
             }
             else {
-                event = new Event(element, eventType);
+                event = new Event(node, eventType);
+            }
+
+            // This is the same as DomElement.fireEvent() and was copied
+            // here so it could be used with HtmlPage.
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Firing " + event);
             }
-            final ScriptResult result = element.fireEvent(event);
+
+            final EventTarget jsNode = node.getScriptableObject();
+            final ContextFactory cf = ((JavaScriptEngine) getWebClient().getJavaScriptEngine()).getContextFactory();
+            final ScriptResult result = cf.call(cx -> jsNode.fireEvent(event));
+
             if (!isOnbeforeunloadAccepted(this, event, result)) {
                 return false;
             }
@@ -1253,7 +1264,12 @@ private boolean executeEventHandlersIfNeeded(final String eventType) {
                     else {
                         event = new Event(frame, eventType);
                     }
-                    final ScriptResult result = ((Node) frame.getScriptableObject()).executeEventLocally(event);
+
+                    // This fires the "load" event for the <frame> element which, like all non-window
+                    // load events, propagates up to Document but not Window.  The "load" event for
+                    // <frameset> on the other hand, like that of <body>, is handled above where it is
+                    // fired against Document and directed to Window.
+                    final ScriptResult result = frame.fireEvent(event);
                     if (!isOnbeforeunloadAccepted((HtmlPage) frame.getPage(), event, result)) {
                         return false;
                     }
```


---

Sent from sourceforge.net because htm...@li... is subscribed to https://sourceforge.net/p/htmlunit/bugs/

To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/htmlunit/admin/bugs/options.  Or, if this is a mailing list, you can unsubscribe from the mailing list.

[HtmlUnit] SVN: [15523] trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/ selenium/TypingTest.java

[<rb...@us...>]

Revision: 15523
          http://sourceforge.net/p/htmlunit/code/15523
Author:   rbri
Date:     2018-08-16 06:41:09 +0000 (Thu, 16 Aug 2018)
Log Message:
-----------
fix expectations for real browsers

Modified Paths:
--------------
    trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/selenium/TypingTest.java

Modified: trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/selenium/TypingTest.java
===================================================================
--- trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/selenium/TypingTest.java	2018-08-16 06:25:35 UTC (rev 15522)
+++ trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/selenium/TypingTest.java	2018-08-16 06:41:09 UTC (rev 15523)
@@ -343,9 +343,11 @@
      */
     @Test
     @Alerts(DEFAULT = {"keydown (target) keyup (target) keyup (body)",
-            "keydown (target) keyup (target) keyup (body) keydown (target) a pressed; removing"},
+                "keydown (target) keyup (target) keyup (body) keydown (target) a pressed; removing"},
             CHROME = {"keydown (target) keyup (target) keyup (body)",
-            "keydown (target) keyup (target) keyup (body) keydown (target) a pressed; removing keyup (body)"})
+                "keydown (target) keyup (target) keyup (body) keydown (target) a pressed; removing keyup (body)"},
+            FF60 = {"keydown (target) keyup (target) keyup (body)",
+                "keydown (target) keyup (target) keyup (body) keydown (target) a pressed; removing keyup (body)"})
     public void canSafelyTypeOnElementThatIsRemovedFromTheDomOnKeyPress() {
         final WebDriver driver = getWebDriver("/key_tests/remove_on_keypress.html");
 

[HtmlUnit] [htmlunit:bugs] #1984 Reworking the JS Event listeners implementation

[RBRi <rb...@us...>]

Will have a look at some of the test cases also, maybe i can migrate some to WebDriverTestCase and see if the expectations are still true.

But there is this EventTest - At the moment i have no idea why real chrome is different is some tests from the other browsers and in some not. I can't see the difference in the test cases - any idea?


---

** [bugs:#1984] Reworking the JS Event listeners implementation**

**Status:** accepted
**Group:** Latest SVN
**Created:** Tue Aug 14, 2018 01:38 PM UTC by Atsushi Nakagawa
**Last Updated:** Thu Aug 16, 2018 06:27 AM UTC
**Owner:** RBRi
**Attachments:**

- [EventListenersContainer.java](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventListenersContainer.java) (16.9 kB; application/octet-stream)
- [EventListenersContainer.java-2.32.txt](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventListenersContainer.java-2.32.txt) (15.5 kB; text/plain)
- [EventTarget.java](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventTarget.java) (14.7 kB; application/octet-stream)
- [EventTarget.java-2.32.txt](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventTarget.java-2.32.txt) (13.4 kB; text/plain)


### Problem in brief

Thorough testing has revealed event listeners in HtmlUnit are implemented quite liberally in a few places with regards to how they should work.  It's somewhat surprising they're working so well in practice.

This bug report attempts to address the problems and tackle at fixing them.  Scope includes `xyz.addListenerEvent('foo', function () { ... })` listeners and `xyz.onfoo = function () { ... }` property handlers.

##### Problems noticed

* Ordering of listeners called during the *at target* phase should honour the order in which the listeners were added, but does not.
* Ordering of property handlers such as `window.onload` should honour the timing at which the property was set, with respect to other event listeners, but does not.
* Events where `event.bubbles` is `false` should not have a *bubbling* phase, but does.
* The load event for `Window`should only traverse `Window` but traverses `Window, Document` instead.
* The load event for everything else should traverse from `Document` to all levels down to the element in question, but traverses the element in question only.  (Every place using `EventTarget.executeEventLocally()` potentially applies to this and should probably be using the standard `EventTarget.fireEvent()` instead.)
* The load event for the `<frame>` element should be treated like  "everything else" above, but is not.  (On the other hand, the `onload` property for `<body>` and `<frameset>` is correctly tied to `Window`.)
* The propagation path of events should not be affected by changes to the DOM tree by intermediate listeners, and therefore the *bubbling* phase should always traverse the same nodes as the *capturing* phase only in reverse, but is and does not.
* The return value of event listeners should be ignored (really? it looks that way.. maybe it was different in older IE) and only that of the property handler should be used, but is not.

### Test cases

There are five tests which I'm putting in the comments because it's quite long.

* `test_onload.html`
* `test_frame.html`
* `test_click.html`
* `test_nested_click.html`
* `test_event_return_value.html`

These extra two I'm including but haven't fixed yet.  They're limited to the load/error behaviour of `<script>` and `<img>` and require fixing up some implementation in `HtmlScript` and `HtmlImage`.

* `test_script_onload.html`
* `test_img_onload.html`

### Possible fix

The fix spans three files:

* `html/HtmlPage.java`
* `javascript/host/event/EventListenersContainer.java`
* `javascript/host/event/EventTarget.java`

Changes to `HtmlPage` is the diff below. 

The changes for `EventListenersContainer.java` and `EventTarget.java` are rather extensive so I'm attaching the modified files along with their respective base versions suffixed with `-2.32.txt`.

```diff
--- a/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
+++ b/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
@@ -86,15 +86,16 @@
 import com.gargoylesoftware.htmlunit.javascript.PostponedAction;
 import com.gargoylesoftware.htmlunit.javascript.SimpleScriptable;
 import com.gargoylesoftware.htmlunit.javascript.host.Window;
-import com.gargoylesoftware.htmlunit.javascript.host.dom.Node;
 import com.gargoylesoftware.htmlunit.javascript.host.event.BeforeUnloadEvent;
 import com.gargoylesoftware.htmlunit.javascript.host.event.Event;
+import com.gargoylesoftware.htmlunit.javascript.host.event.EventTarget;
 import com.gargoylesoftware.htmlunit.javascript.host.html.HTMLDocument;
 import com.gargoylesoftware.htmlunit.protocol.javascript.JavaScriptURLConnection;
 import com.gargoylesoftware.htmlunit.util.EncodingSniffer;
 import com.gargoylesoftware.htmlunit.util.UrlUtils;
 
 import net.sourceforge.htmlunit.corejs.javascript.Context;
+import net.sourceforge.htmlunit.corejs.javascript.ContextFactory;
 import net.sourceforge.htmlunit.corejs.javascript.Function;
 import net.sourceforge.htmlunit.corejs.javascript.Script;
 import net.sourceforge.htmlunit.corejs.javascript.Scriptable;
@@ -1214,18 +1215,28 @@ private boolean executeEventHandlersIfNeeded(final String eventType) {
         // Execute the specified event on the document element.
         final WebWindow window = getEnclosingWindow();
         if (window.getScriptableObject() instanceof Window) {
-            final DomElement element = getDocumentElement();
-            if (element == null) { // happens for instance if document.documentElement has been removed from parent
-                return true;
-            }
+            // We need the 'Document' node for these load events but getDocumentElement() returns
+            // <html> (HtmlHtml) which is one below that.  The 'Document' node is incidentally just
+            // us (HtmlPage).  (Some tidbits at https://www.w3.org/TR/DOM-Level-3-Events/#event-flow)
+            final DomNode node = this;
             final Event event;
             if (eventType.equals(Event.TYPE_BEFORE_UNLOAD)) {
-                event = new BeforeUnloadEvent(element, eventType);
+                event = new BeforeUnloadEvent(node, eventType);
             }
             else {
-                event = new Event(element, eventType);
+                event = new Event(node, eventType);
+            }
+
+            // This is the same as DomElement.fireEvent() and was copied
+            // here so it could be used with HtmlPage.
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Firing " + event);
             }
-            final ScriptResult result = element.fireEvent(event);
+
+            final EventTarget jsNode = node.getScriptableObject();
+            final ContextFactory cf = ((JavaScriptEngine) getWebClient().getJavaScriptEngine()).getContextFactory();
+            final ScriptResult result = cf.call(cx -> jsNode.fireEvent(event));
+
             if (!isOnbeforeunloadAccepted(this, event, result)) {
                 return false;
             }
@@ -1253,7 +1264,12 @@ private boolean executeEventHandlersIfNeeded(final String eventType) {
                     else {
                         event = new Event(frame, eventType);
                     }
-                    final ScriptResult result = ((Node) frame.getScriptableObject()).executeEventLocally(event);
+
+                    // This fires the "load" event for the <frame> element which, like all non-window
+                    // load events, propagates up to Document but not Window.  The "load" event for
+                    // <frameset> on the other hand, like that of <body>, is handled above where it is
+                    // fired against Document and directed to Window.
+                    final ScriptResult result = frame.fireEvent(event);
                     if (!isOnbeforeunloadAccepted((HtmlPage) frame.getPage(), event, result)) {
                         return false;
                     }
```


---

Sent from sourceforge.net because htm...@li... is subscribed to https://sourceforge.net/p/htmlunit/bugs/

To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/htmlunit/admin/bugs/options.  Or, if this is a mailing list, you can unsubscribe from the mailing list.

[HtmlUnit] [htmlunit:bugs] #1984 Reworking the JS Event listeners implementation

[RBRi <rb...@us...>]

Have commited you last change also and made some imporvements on the tests. Looks a bit better now.
But our test suite still reports many problems.


---

** [bugs:#1984] Reworking the JS Event listeners implementation**

**Status:** accepted
**Group:** Latest SVN
**Created:** Tue Aug 14, 2018 01:38 PM UTC by Atsushi Nakagawa
**Last Updated:** Thu Aug 16, 2018 02:03 AM UTC
**Owner:** RBRi
**Attachments:**

- [EventListenersContainer.java](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventListenersContainer.java) (16.9 kB; application/octet-stream)
- [EventListenersContainer.java-2.32.txt](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventListenersContainer.java-2.32.txt) (15.5 kB; text/plain)
- [EventTarget.java](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventTarget.java) (14.7 kB; application/octet-stream)
- [EventTarget.java-2.32.txt](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventTarget.java-2.32.txt) (13.4 kB; text/plain)


### Problem in brief

Thorough testing has revealed event listeners in HtmlUnit are implemented quite liberally in a few places with regards to how they should work.  It's somewhat surprising they're working so well in practice.

This bug report attempts to address the problems and tackle at fixing them.  Scope includes `xyz.addListenerEvent('foo', function () { ... })` listeners and `xyz.onfoo = function () { ... }` property handlers.

##### Problems noticed

* Ordering of listeners called during the *at target* phase should honour the order in which the listeners were added, but does not.
* Ordering of property handlers such as `window.onload` should honour the timing at which the property was set, with respect to other event listeners, but does not.
* Events where `event.bubbles` is `false` should not have a *bubbling* phase, but does.
* The load event for `Window`should only traverse `Window` but traverses `Window, Document` instead.
* The load event for everything else should traverse from `Document` to all levels down to the element in question, but traverses the element in question only.  (Every place using `EventTarget.executeEventLocally()` potentially applies to this and should probably be using the standard `EventTarget.fireEvent()` instead.)
* The load event for the `<frame>` element should be treated like  "everything else" above, but is not.  (On the other hand, the `onload` property for `<body>` and `<frameset>` is correctly tied to `Window`.)
* The propagation path of events should not be affected by changes to the DOM tree by intermediate listeners, and therefore the *bubbling* phase should always traverse the same nodes as the *capturing* phase only in reverse, but is and does not.
* The return value of event listeners should be ignored (really? it looks that way.. maybe it was different in older IE) and only that of the property handler should be used, but is not.

### Test cases

There are five tests which I'm putting in the comments because it's quite long.

* `test_onload.html`
* `test_frame.html`
* `test_click.html`
* `test_nested_click.html`
* `test_event_return_value.html`

These extra two I'm including but haven't fixed yet.  They're limited to the load/error behaviour of `<script>` and `<img>` and require fixing up some implementation in `HtmlScript` and `HtmlImage`.

* `test_script_onload.html`
* `test_img_onload.html`

### Possible fix

The fix spans three files:

* `html/HtmlPage.java`
* `javascript/host/event/EventListenersContainer.java`
* `javascript/host/event/EventTarget.java`

Changes to `HtmlPage` is the diff below. 

The changes for `EventListenersContainer.java` and `EventTarget.java` are rather extensive so I'm attaching the modified files along with their respective base versions suffixed with `-2.32.txt`.

```diff
--- a/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
+++ b/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
@@ -86,15 +86,16 @@
 import com.gargoylesoftware.htmlunit.javascript.PostponedAction;
 import com.gargoylesoftware.htmlunit.javascript.SimpleScriptable;
 import com.gargoylesoftware.htmlunit.javascript.host.Window;
-import com.gargoylesoftware.htmlunit.javascript.host.dom.Node;
 import com.gargoylesoftware.htmlunit.javascript.host.event.BeforeUnloadEvent;
 import com.gargoylesoftware.htmlunit.javascript.host.event.Event;
+import com.gargoylesoftware.htmlunit.javascript.host.event.EventTarget;
 import com.gargoylesoftware.htmlunit.javascript.host.html.HTMLDocument;
 import com.gargoylesoftware.htmlunit.protocol.javascript.JavaScriptURLConnection;
 import com.gargoylesoftware.htmlunit.util.EncodingSniffer;
 import com.gargoylesoftware.htmlunit.util.UrlUtils;
 
 import net.sourceforge.htmlunit.corejs.javascript.Context;
+import net.sourceforge.htmlunit.corejs.javascript.ContextFactory;
 import net.sourceforge.htmlunit.corejs.javascript.Function;
 import net.sourceforge.htmlunit.corejs.javascript.Script;
 import net.sourceforge.htmlunit.corejs.javascript.Scriptable;
@@ -1214,18 +1215,28 @@ private boolean executeEventHandlersIfNeeded(final String eventType) {
         // Execute the specified event on the document element.
         final WebWindow window = getEnclosingWindow();
         if (window.getScriptableObject() instanceof Window) {
-            final DomElement element = getDocumentElement();
-            if (element == null) { // happens for instance if document.documentElement has been removed from parent
-                return true;
-            }
+            // We need the 'Document' node for these load events but getDocumentElement() returns
+            // <html> (HtmlHtml) which is one below that.  The 'Document' node is incidentally just
+            // us (HtmlPage).  (Some tidbits at https://www.w3.org/TR/DOM-Level-3-Events/#event-flow)
+            final DomNode node = this;
             final Event event;
             if (eventType.equals(Event.TYPE_BEFORE_UNLOAD)) {
-                event = new BeforeUnloadEvent(element, eventType);
+                event = new BeforeUnloadEvent(node, eventType);
             }
             else {
-                event = new Event(element, eventType);
+                event = new Event(node, eventType);
+            }
+
+            // This is the same as DomElement.fireEvent() and was copied
+            // here so it could be used with HtmlPage.
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Firing " + event);
             }
-            final ScriptResult result = element.fireEvent(event);
+
+            final EventTarget jsNode = node.getScriptableObject();
+            final ContextFactory cf = ((JavaScriptEngine) getWebClient().getJavaScriptEngine()).getContextFactory();
+            final ScriptResult result = cf.call(cx -> jsNode.fireEvent(event));
+
             if (!isOnbeforeunloadAccepted(this, event, result)) {
                 return false;
             }
@@ -1253,7 +1264,12 @@ private boolean executeEventHandlersIfNeeded(final String eventType) {
                     else {
                         event = new Event(frame, eventType);
                     }
-                    final ScriptResult result = ((Node) frame.getScriptableObject()).executeEventLocally(event);
+
+                    // This fires the "load" event for the <frame> element which, like all non-window
+                    // load events, propagates up to Document but not Window.  The "load" event for
+                    // <frameset> on the other hand, like that of <body>, is handled above where it is
+                    // fired against Document and directed to Window.
+                    final ScriptResult result = frame.fireEvent(event);
                     if (!isOnbeforeunloadAccepted((HtmlPage) frame.getPage(), event, result)) {
                         return false;
                     }
```


---

Sent from sourceforge.net because htm...@li... is subscribed to https://sourceforge.net/p/htmlunit/bugs/

To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/htmlunit/admin/bugs/options.  Or, if this is a mailing list, you can unsubscribe from the mailing list.

[HtmlUnit] SVN: [15522] trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/ html/HtmlPage.java

[<rb...@us...>]

Revision: 15522
          http://sourceforge.net/p/htmlunit/code/15522
Author:   rbri
Date:     2018-08-16 06:25:35 +0000 (Thu, 16 Aug 2018)
Log Message:
-----------
next step in event refactoring (wip)

Modified Paths:
--------------
    trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java

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-16 06:25:01 UTC (rev 15521)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java	2018-08-16 06:25:35 UTC (rev 15522)
@@ -246,6 +246,9 @@
                 }
             }
         }
+
+        executeEventHandlersIfNeeded(Event.TYPE_DOM_DOCUMENT_LOADED);
+
         loadFrames();
 
         // don't set the ready state if we really load the blank page into the window
@@ -258,7 +261,6 @@
             getDocumentElement().setReadyState(READY_STATE_COMPLETE);
         }
 
-        executeEventHandlersIfNeeded(Event.TYPE_DOM_DOCUMENT_LOADED);
         executeDeferredScriptsIfNeeded();
         setReadyStateOnDeferredScriptsIfNeeded();
 

[HtmlUnit] SVN: [15521] trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/ javascript/host/Window3Test.java

[<rb...@us...>]

Revision: 15521
          http://sourceforge.net/p/htmlunit/code/15521
Author:   rbri
Date:     2018-08-16 06:25:01 +0000 (Thu, 16 Aug 2018)
Log Message:
-----------
make tests more robust

Modified Paths:
--------------
    trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/Window3Test.java

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 08:09:56 UTC (rev 15520)
+++ trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/Window3Test.java	2018-08-16 06:25:01 UTC (rev 15521)
@@ -1627,7 +1627,7 @@
             + "<html><head>\n"
             + "<script>\n"
             + "  function log(msg) {\n"
-            + "    document.getElementById('log').value += msg + '\\n';\n"
+            + "    window.parent.document.title += msg + ';';\n"
             + "  }\n"
 
             // These 'load' events and 'onload' property below target 'document' when fired
@@ -1682,12 +1682,11 @@
                     + "function (event) { var x = event; "
                         + "window.setTimeout(function () { log('after', x.eventPhase) }, 100) }, true)\n"
             + "</script>\n"
-            + "  <textarea id='log' rows=40 cols=80></textarea>\n"
             + "</body></html>";
 
         final WebDriver driver = loadPage2(html);
         Thread.sleep(200);
-        final String text = driver.findElement(By.id("log")).getAttribute("value").trim().replaceAll("\r", "");
+        final String text = driver.getTitle().trim().replaceAll(";", "\n").trim();
         assertEquals(String.join("\n", getExpectedAlerts()), text);
     }
 
@@ -1699,17 +1698,27 @@
      * @throws Exception if the test fails
      */
     @Test
-    @Alerts(DEFAULT = {"document DOMContentLoaded",
+    @Alerts(DEFAULT = {"document at load capture",
+                        "element 1 onload",
+                        "window at error capture",
+                        "document at error capture",
+                        "element 2 onerror",
+                        "document DOMContentLoaded",
                         "window DOMContentLoaded",
                         "window at load",
                         "window at load capture",
                         "body onload"},
-            IE = {"document DOMContentLoaded",
+            IE = {"document at load capture",
+                        "element 1 onload",
+                        "window at error capture",
+                        "document at error capture",
+                        "element 2 onerror",
+                        "document DOMContentLoaded",
                         "window DOMContentLoaded",
                         "window at load",
                         "window at load capture",
-                        "document at load capture",
-                        "body onload"})
+                        "body onload",
+                        "document at load capture"})
     public void onloadScript() throws Exception {
         getMockWebConnection().setResponse(URL_SECOND, "");
 
@@ -1717,7 +1726,7 @@
             + "<html><head>\n"
             + "<script>\n"
             + "  function log(msg) {\n"
-            + "    document.getElementById('log').value += msg + '\\n';\n"
+            + "    window.parent.document.title += msg + ';';\n"
             + "  }\n"
 
             + "  window.addEventListener('load', function () { log('window at load') })\n"
@@ -1739,13 +1748,11 @@
                                         + "onerror='log(\"element 1 onerror\")'></script>\n"
             + "  <script src='missing.txt' onload='log(\"element 2 onload\")' "
                                         + "onerror='log(\"element 2 onerror\")'></script>\n"
-
-            + "  <textarea id='log' rows=40 cols=80></textarea>\n"
             + "</body></html>";
 
         final WebDriver driver = loadPage2(html);
         Thread.sleep(200);
-        final String text = driver.findElement(By.id("log")).getAttribute("value").trim().replaceAll("\r", "");
+        final String text = driver.getTitle().trim().replaceAll(";", "\n").trim();
         assertEquals(String.join("\n", getExpectedAlerts()), text);
     }
 
@@ -1800,7 +1807,7 @@
             + "<html><head>\n"
             + "<script>\n"
             + "  function log(msg) {\n"
-            + "    document.getElementById('log').value += msg + '\\n';\n"
+            + "    window.parent.document.title += msg + ';';\n"
             + "  }\n"
 
             + "  window.addEventListener('load', function () { log('window at load') })\n"
@@ -1823,12 +1830,11 @@
             + "  <img src='' onload='log(\"element 2 onload\")' "
                                         + "onerror='log(\"element 2 onerror\")'>\n"
 
-            + "  <textarea id='log' rows=40 cols=80></textarea>\n"
             + "</body></html>";
 
         final WebDriver driver = loadPage2(html);
         Thread.sleep(200);
-        final String text = driver.findElement(By.id("log")).getAttribute("value").trim().replaceAll("\r", "");
+        final String text = driver.getTitle().trim().replaceAll(";", "\n").trim();
         assertEquals(String.join("\n", getExpectedAlerts()), text);
     }
 
@@ -1991,13 +1997,12 @@
             + "<html><head>\n"
             + "<script>\n"
             + "  function log(msg) {\n"
-            + "    document.getElementById('log').value += msg + '\\n';\n"
+            + "    window.parent.document.title += msg + ';';\n"
             + "  }\n"
             + "</script>\n"
             + "</head>\n"
             + "<body>\n"
             + "  <input id='tester' type='button' value='test' onclick='log(\"onclick\")'>\n"
-            + "  <textarea id='log' rows=40 cols=80></textarea>\n"
 
             + "<script>\n"
             + "  window.addEventListener('click', function () { log('window at click 1') })\n"
@@ -2016,7 +2021,7 @@
         final WebDriver driver = loadPage2(html);
         driver.findElement(By.id("tester")).click();
 
-        final String text = driver.findElement(By.id("log")).getAttribute("value").trim().replaceAll("\r", "");
+        final String text = driver.getTitle().trim().replaceAll(";", "\n").trim();
         assertEquals(String.join("\n", getExpectedAlerts()), text);
     }
 
@@ -2051,7 +2056,7 @@
             + "<html><head>\n"
             + "<script>\n"
             + "  function log(msg) {\n"
-            + "    document.getElementById('log').value += msg + '\\n';\n"
+            + "    window.parent.document.title += msg + ';';\n"
             + "  }\n"
             + "</script>\n"
             + "</head>\n"
@@ -2063,8 +2068,6 @@
             + "    </div>\n"
             + "  </div>\n"
 
-            + "  <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"
@@ -2094,7 +2097,7 @@
         final WebDriver driver = loadPage2(html);
         driver.findElement(By.id("d3")).click();
 
-        final String text = driver.findElement(By.id("log")).getAttribute("value").trim().replaceAll("\r", "");
+        final String text = driver.getTitle().trim().replaceAll(";", "\n").trim();
         assertEquals(String.join("\n", getExpectedAlerts()), text);
     }
 
@@ -2126,7 +2129,7 @@
             + "<html><head>\n"
             + "<script>\n"
             + "  function log(msg) {\n"
-            + "    document.getElementById('log').value += msg + '\\n';\n"
+            + "    window.parent.document.title += msg + ';';\n"
             + "  }\n"
 
             + "  function detachAndClick() {\n"
@@ -2147,8 +2150,6 @@
             + "  </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"
@@ -2179,7 +2180,7 @@
         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", "");
+        final String text = driver.getTitle().trim().replaceAll(";", "\n").trim();
         assertEquals(String.join("\n", getExpectedAlerts()), text);
     }
 
@@ -2200,7 +2201,7 @@
             + "<html><head>\n"
             + "<script>\n"
             + "  function log(msg) {\n"
-            + "    document.getElementById('log').value += msg + '\\n';\n"
+            + "    window.parent.document.title += msg + ';';\n"
             + "  }\n"
             + "</script>\n"
             + "</head>\n"
@@ -2238,7 +2239,7 @@
         driver.findElement(By.id("a1")).click();
         driver.findElement(By.id("a2")).click();
 
-        final String text = driver.findElement(By.id("log")).getAttribute("value").trim().replaceAll("\r", "");
+        final String text = driver.getTitle().trim().replaceAll(";", "\n").trim();
         assertEquals(String.join("\n", getExpectedAlerts()), text);
     }
 }

[HtmlUnit] [htmlunit:bugs] #1984 Reworking the JS Event listeners implementation

[RBRi <rb...@us...>]

Or simply you can have a look at he build server https://ci.canoo.com/teamcity/project.html?projectId=HtmlUnit&branch_HtmlUnit=__all_branches__ (Login as Guest)


---

** [bugs:#1984] Reworking the JS Event listeners implementation**

**Status:** accepted
**Group:** Latest SVN
**Created:** Tue Aug 14, 2018 01:38 PM UTC by Atsushi Nakagawa
**Last Updated:** Wed Aug 15, 2018 10:02 AM UTC
**Owner:** RBRi
**Attachments:**

- [EventListenersContainer.java](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventListenersContainer.java) (16.9 kB; application/octet-stream)
- [EventListenersContainer.java-2.32.txt](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventListenersContainer.java-2.32.txt) (15.5 kB; text/plain)
- [EventTarget.java](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventTarget.java) (14.7 kB; application/octet-stream)
- [EventTarget.java-2.32.txt](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventTarget.java-2.32.txt) (13.4 kB; text/plain)


### Problem in brief

Thorough testing has revealed event listeners in HtmlUnit are implemented quite liberally in a few places with regards to how they should work.  It's somewhat surprising they're working so well in practice.

This bug report attempts to address the problems and tackle at fixing them.  Scope includes `xyz.addListenerEvent('foo', function () { ... })` listeners and `xyz.onfoo = function () { ... }` property handlers.

##### Problems noticed

* Ordering of listeners called during the *at target* phase should honour the order in which the listeners were added, but does not.
* Ordering of property handlers such as `window.onload` should honour the timing at which the property was set, with respect to other event listeners, but does not.
* Events where `event.bubbles` is `false` should not have a *bubbling* phase, but does.
* The load event for `Window`should only traverse `Window` but traverses `Window, Document` instead.
* The load event for everything else should traverse from `Document` to all levels down to the element in question, but traverses the element in question only.  (Every place using `EventTarget.executeEventLocally()` potentially applies to this and should probably be using the standard `EventTarget.fireEvent()` instead.)
* The load event for the `<frame>` element should be treated like  "everything else" above, but is not.  (On the other hand, the `onload` property for `<body>` and `<frameset>` is correctly tied to `Window`.)
* The propagation path of events should not be affected by changes to the DOM tree by intermediate listeners, and therefore the *bubbling* phase should always traverse the same nodes as the *capturing* phase only in reverse, but is and does not.
* The return value of event listeners should be ignored (really? it looks that way.. maybe it was different in older IE) and only that of the property handler should be used, but is not.

### Test cases

There are five tests which I'm putting in the comments because it's quite long.

* `test_onload.html`
* `test_frame.html`
* `test_click.html`
* `test_nested_click.html`
* `test_event_return_value.html`

These extra two I'm including but haven't fixed yet.  They're limited to the load/error behaviour of `<script>` and `<img>` and require fixing up some implementation in `HtmlScript` and `HtmlImage`.

* `test_script_onload.html`
* `test_img_onload.html`

### Possible fix

The fix spans three files:

* `html/HtmlPage.java`
* `javascript/host/event/EventListenersContainer.java`
* `javascript/host/event/EventTarget.java`

Changes to `HtmlPage` is the diff below. 

The changes for `EventListenersContainer.java` and `EventTarget.java` are rather extensive so I'm attaching the modified files along with their respective base versions suffixed with `-2.32.txt`.

```diff
--- a/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
+++ b/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
@@ -86,15 +86,16 @@
 import com.gargoylesoftware.htmlunit.javascript.PostponedAction;
 import com.gargoylesoftware.htmlunit.javascript.SimpleScriptable;
 import com.gargoylesoftware.htmlunit.javascript.host.Window;
-import com.gargoylesoftware.htmlunit.javascript.host.dom.Node;
 import com.gargoylesoftware.htmlunit.javascript.host.event.BeforeUnloadEvent;
 import com.gargoylesoftware.htmlunit.javascript.host.event.Event;
+import com.gargoylesoftware.htmlunit.javascript.host.event.EventTarget;
 import com.gargoylesoftware.htmlunit.javascript.host.html.HTMLDocument;
 import com.gargoylesoftware.htmlunit.protocol.javascript.JavaScriptURLConnection;
 import com.gargoylesoftware.htmlunit.util.EncodingSniffer;
 import com.gargoylesoftware.htmlunit.util.UrlUtils;
 
 import net.sourceforge.htmlunit.corejs.javascript.Context;
+import net.sourceforge.htmlunit.corejs.javascript.ContextFactory;
 import net.sourceforge.htmlunit.corejs.javascript.Function;
 import net.sourceforge.htmlunit.corejs.javascript.Script;
 import net.sourceforge.htmlunit.corejs.javascript.Scriptable;
@@ -1214,18 +1215,28 @@ private boolean executeEventHandlersIfNeeded(final String eventType) {
         // Execute the specified event on the document element.
         final WebWindow window = getEnclosingWindow();
         if (window.getScriptableObject() instanceof Window) {
-            final DomElement element = getDocumentElement();
-            if (element == null) { // happens for instance if document.documentElement has been removed from parent
-                return true;
-            }
+            // We need the 'Document' node for these load events but getDocumentElement() returns
+            // <html> (HtmlHtml) which is one below that.  The 'Document' node is incidentally just
+            // us (HtmlPage).  (Some tidbits at https://www.w3.org/TR/DOM-Level-3-Events/#event-flow)
+            final DomNode node = this;
             final Event event;
             if (eventType.equals(Event.TYPE_BEFORE_UNLOAD)) {
-                event = new BeforeUnloadEvent(element, eventType);
+                event = new BeforeUnloadEvent(node, eventType);
             }
             else {
-                event = new Event(element, eventType);
+                event = new Event(node, eventType);
+            }
+
+            // This is the same as DomElement.fireEvent() and was copied
+            // here so it could be used with HtmlPage.
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Firing " + event);
             }
-            final ScriptResult result = element.fireEvent(event);
+
+            final EventTarget jsNode = node.getScriptableObject();
+            final ContextFactory cf = ((JavaScriptEngine) getWebClient().getJavaScriptEngine()).getContextFactory();
+            final ScriptResult result = cf.call(cx -> jsNode.fireEvent(event));
+
             if (!isOnbeforeunloadAccepted(this, event, result)) {
                 return false;
             }
@@ -1253,7 +1264,12 @@ private boolean executeEventHandlersIfNeeded(final String eventType) {
                     else {
                         event = new Event(frame, eventType);
                     }
-                    final ScriptResult result = ((Node) frame.getScriptableObject()).executeEventLocally(event);
+
+                    // This fires the "load" event for the <frame> element which, like all non-window
+                    // load events, propagates up to Document but not Window.  The "load" event for
+                    // <frameset> on the other hand, like that of <body>, is handled above where it is
+                    // fired against Document and directed to Window.
+                    final ScriptResult result = frame.fireEvent(event);
                     if (!isOnbeforeunloadAccepted((HtmlPage) frame.getPage(), event, result)) {
                         return false;
                     }
```


---

Sent from sourceforge.net because htm...@li... is subscribed to https://sourceforge.net/p/htmlunit/bugs/

To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/htmlunit/admin/bugs/options.  Or, if this is a mailing list, you can unsubscribe from the mailing list.

[HtmlUnit] SVN: [15520] trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/ ExternalTest.java

[<rb...@us...>]

Revision: 15520
          http://sourceforge.net/p/htmlunit/code/15520
Author:   rbri
Date:     2018-08-15 08:09:56 +0000 (Wed, 15 Aug 2018)
Log Message:
-----------
looks like github has changed the page html code

Modified Paths:
--------------
    trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/ExternalTest.java

Modified: trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/ExternalTest.java
===================================================================
--- trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/ExternalTest.java	2018-08-15 08:01:20 UTC (rev 15519)
+++ trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/ExternalTest.java	2018-08-15 08:09:56 UTC (rev 15520)
@@ -125,7 +125,7 @@
         try (WebClient webClient = getWebClient()) {
             try {
                 final HtmlPage page = webClient.getPage("https://github.com/mozilla/geckodriver/releases/latest");
-                final DomNodeList<DomNode> divs = page.querySelectorAll(".release-title");
+                final DomNodeList<DomNode> divs = page.querySelectorAll(".release-header div");
                 assertEquals("Gecko Driver", divs.get(0).asText(), "v" + GECKO_DRIVER_);
             }
             catch (final FailingHttpStatusCodeException e) {

[HtmlUnit] [htmlunit:bugs] #1984 Reworking the JS Event listeners implementation

[RBRi <rb...@us...>]

Have added all your cases as test cases to Window3Test; the expectations are the ones i got when runnung unsing the rela browsers (switching test.properties to real browsers).

Have added all your patches -> please verigy the changes and the tests.

Now the build is not happy, some other test cases are failing - any idea


---

** [bugs:#1984] Reworking the JS Event listeners implementation**

**Status:** accepted
**Group:** Latest SVN
**Created:** Tue Aug 14, 2018 01:38 PM UTC by Atsushi Nakagawa
**Last Updated:** Wed Aug 15, 2018 04:07 AM UTC
**Owner:** RBRi
**Attachments:**

- [EventListenersContainer.java](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventListenersContainer.java) (16.9 kB; application/octet-stream)
- [EventListenersContainer.java-2.32.txt](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventListenersContainer.java-2.32.txt) (15.5 kB; text/plain)
- [EventTarget.java](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventTarget.java) (14.7 kB; application/octet-stream)
- [EventTarget.java-2.32.txt](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventTarget.java-2.32.txt) (13.4 kB; text/plain)


### Problem in brief

Thorough testing has revealed event listeners in HtmlUnit are implemented quite liberally in a few places with regards to how they should work.  It's somewhat surprising they're working so well in practice.

This bug report attempts to address the problems and tackle at fixing them.  Scope includes `xyz.addListenerEvent('foo', function () { ... })` listeners and `xyz.onfoo = function () { ... }` property handlers.

##### Problems noticed

* Ordering of listeners called during the *at target* phase should honour the order in which the listeners were added, but does not.
* Ordering of property handlers such as `window.onload` should honour the timing at which the property was set, with respect to other event listeners, but does not.
* Events where `event.bubbles` is `false` should not have a *bubbling* phase, but does.
* The load event for `Window`should only traverse `Window` but traverses `Window, Document` instead.
* The load event for everything else should traverse from `Document` to all levels down to the element in question, but traverses the element in question only.  (Every place using `EventTarget.executeEventLocally()` potentially applies to this and should probably be using the standard `EventTarget.fireEvent()` instead.)
* The load event for the `<frame>` element should be treated like  "everything else" above, but is not.  (On the other hand, the `onload` property for `<body>` and `<frameset>` is correctly tied to `Window`.)
* The propagation path of events should not be affected by changes to the DOM tree by intermediate listeners, and therefore the *bubbling* phase should always traverse the same nodes as the *capturing* phase only in reverse, but is and does not.
* The return value of event listeners should be ignored (really? it looks that way.. maybe it was different in older IE) and only that of the property handler should be used, but is not.

### Test cases

There are five tests which I'm putting in the comments because it's quite long.

* `test_onload.html`
* `test_frame.html`
* `test_click.html`
* `test_nested_click.html`
* `test_event_return_value.html`

These extra two I'm including but haven't fixed yet.  They're limited to the load/error behaviour of `<script>` and `<img>` and require fixing up some implementation in `HtmlScript` and `HtmlImage`.

* `test_script_onload.html`
* `test_img_onload.html`

### Possible fix

The fix spans three files:

* `html/HtmlPage.java`
* `javascript/host/event/EventListenersContainer.java`
* `javascript/host/event/EventTarget.java`

Changes to `HtmlPage` is the diff below. 

The changes for `EventListenersContainer.java` and `EventTarget.java` are rather extensive so I'm attaching the modified files along with their respective base versions suffixed with `-2.32.txt`.

```diff
--- a/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
+++ b/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
@@ -86,15 +86,16 @@
 import com.gargoylesoftware.htmlunit.javascript.PostponedAction;
 import com.gargoylesoftware.htmlunit.javascript.SimpleScriptable;
 import com.gargoylesoftware.htmlunit.javascript.host.Window;
-import com.gargoylesoftware.htmlunit.javascript.host.dom.Node;
 import com.gargoylesoftware.htmlunit.javascript.host.event.BeforeUnloadEvent;
 import com.gargoylesoftware.htmlunit.javascript.host.event.Event;
+import com.gargoylesoftware.htmlunit.javascript.host.event.EventTarget;
 import com.gargoylesoftware.htmlunit.javascript.host.html.HTMLDocument;
 import com.gargoylesoftware.htmlunit.protocol.javascript.JavaScriptURLConnection;
 import com.gargoylesoftware.htmlunit.util.EncodingSniffer;
 import com.gargoylesoftware.htmlunit.util.UrlUtils;
 
 import net.sourceforge.htmlunit.corejs.javascript.Context;
+import net.sourceforge.htmlunit.corejs.javascript.ContextFactory;
 import net.sourceforge.htmlunit.corejs.javascript.Function;
 import net.sourceforge.htmlunit.corejs.javascript.Script;
 import net.sourceforge.htmlunit.corejs.javascript.Scriptable;
@@ -1214,18 +1215,28 @@ private boolean executeEventHandlersIfNeeded(final String eventType) {
         // Execute the specified event on the document element.
         final WebWindow window = getEnclosingWindow();
         if (window.getScriptableObject() instanceof Window) {
-            final DomElement element = getDocumentElement();
-            if (element == null) { // happens for instance if document.documentElement has been removed from parent
-                return true;
-            }
+            // We need the 'Document' node for these load events but getDocumentElement() returns
+            // <html> (HtmlHtml) which is one below that.  The 'Document' node is incidentally just
+            // us (HtmlPage).  (Some tidbits at https://www.w3.org/TR/DOM-Level-3-Events/#event-flow)
+            final DomNode node = this;
             final Event event;
             if (eventType.equals(Event.TYPE_BEFORE_UNLOAD)) {
-                event = new BeforeUnloadEvent(element, eventType);
+                event = new BeforeUnloadEvent(node, eventType);
             }
             else {
-                event = new Event(element, eventType);
+                event = new Event(node, eventType);
+            }
+
+            // This is the same as DomElement.fireEvent() and was copied
+            // here so it could be used with HtmlPage.
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Firing " + event);
             }
-            final ScriptResult result = element.fireEvent(event);
+
+            final EventTarget jsNode = node.getScriptableObject();
+            final ContextFactory cf = ((JavaScriptEngine) getWebClient().getJavaScriptEngine()).getContextFactory();
+            final ScriptResult result = cf.call(cx -> jsNode.fireEvent(event));
+
             if (!isOnbeforeunloadAccepted(this, event, result)) {
                 return false;
             }
@@ -1253,7 +1264,12 @@ private boolean executeEventHandlersIfNeeded(final String eventType) {
                     else {
                         event = new Event(frame, eventType);
                     }
-                    final ScriptResult result = ((Node) frame.getScriptableObject()).executeEventLocally(event);
+
+                    // This fires the "load" event for the <frame> element which, like all non-window
+                    // load events, propagates up to Document but not Window.  The "load" event for
+                    // <frameset> on the other hand, like that of <body>, is handled above where it is
+                    // fired against Document and directed to Window.
+                    final ScriptResult result = frame.fireEvent(event);
                     if (!isOnbeforeunloadAccepted((HtmlPage) frame.getPage(), event, result)) {
                         return false;
                     }
```


---

Sent from sourceforge.net because htm...@li... is subscribed to https://sourceforge.net/p/htmlunit/bugs/

To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/htmlunit/admin/bugs/options.  Or, if this is a mailing list, you can unsubscribe from the mailing list.

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

[<rb...@us...>]

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"

[HtmlUnit] SVN: [15518] trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/ javascript/host/worker/DedicatedWorkerGlobalScope.java

[<rb...@us...>]

Revision: 15518
          http://sourceforge.net/p/htmlunit/code/15518
Author:   rbri
Date:     2018-08-15 07:14:08 +0000 (Wed, 15 Aug 2018)
Log Message:
-----------
fix compile error

Modified Paths:
--------------
    trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/worker/DedicatedWorkerGlobalScope.java

Modified: trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/worker/DedicatedWorkerGlobalScope.java
===================================================================
--- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/worker/DedicatedWorkerGlobalScope.java	2018-08-14 19:17:44 UTC (rev 15517)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/worker/DedicatedWorkerGlobalScope.java	2018-08-15 07:14:08 UTC (rev 15518)
@@ -125,7 +125,7 @@
             public Object run(final Context cx) {
                 worker_.getEventListenersContainer().executeCapturingListeners(event, null);
                 final Object[] args = new Object[] {event};
-                return worker_.getEventListenersContainer().executeBubblingListeners(event, args, args);
+                return worker_.getEventListenersContainer().executeBubblingListeners(event, args);
             }
         };
 

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

[<rb...@us...>]

Revision: 15517
          http://sourceforge.net/p/htmlunit/code/15517
Author:   rbri
Date:     2018-08-14 19:17:44 +0000 (Tue, 14 Aug 2018)
Log Message:
-----------
give this fix a try (wip)

Modified Paths:
--------------
    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/html/HtmlPage.java
===================================================================
--- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java	2018-08-12 12:32:23 UTC (rev 15516)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java	2018-08-14 19:17:44 UTC (rev 15517)
@@ -85,9 +85,9 @@
 import com.gargoylesoftware.htmlunit.javascript.PostponedAction;
 import com.gargoylesoftware.htmlunit.javascript.SimpleScriptable;
 import com.gargoylesoftware.htmlunit.javascript.host.Window;
-import com.gargoylesoftware.htmlunit.javascript.host.dom.Node;
 import com.gargoylesoftware.htmlunit.javascript.host.event.BeforeUnloadEvent;
 import com.gargoylesoftware.htmlunit.javascript.host.event.Event;
+import com.gargoylesoftware.htmlunit.javascript.host.event.EventTarget;
 import com.gargoylesoftware.htmlunit.javascript.host.html.HTMLDocument;
 import com.gargoylesoftware.htmlunit.protocol.javascript.JavaScriptURLConnection;
 import com.gargoylesoftware.htmlunit.util.EncodingSniffer;
@@ -94,6 +94,7 @@
 import com.gargoylesoftware.htmlunit.util.UrlUtils;
 
 import net.sourceforge.htmlunit.corejs.javascript.Context;
+import net.sourceforge.htmlunit.corejs.javascript.ContextFactory;
 import net.sourceforge.htmlunit.corejs.javascript.Function;
 import net.sourceforge.htmlunit.corejs.javascript.Script;
 import net.sourceforge.htmlunit.corejs.javascript.Scriptable;
@@ -1206,18 +1207,23 @@
         // Execute the specified event on the document element.
         final WebWindow window = getEnclosingWindow();
         if (window.getScriptableObject() instanceof Window) {
-            final DomElement element = getDocumentElement();
-            if (element == null) { // happens for instance if document.documentElement has been removed from parent
-                return true;
-            }
             final Event event;
             if (eventType.equals(Event.TYPE_BEFORE_UNLOAD)) {
-                event = new BeforeUnloadEvent(element, eventType);
+                event = new BeforeUnloadEvent(this, eventType);
             }
             else {
-                event = new Event(element, eventType);
+                event = new Event(this, eventType);
             }
-            final ScriptResult result = element.fireEvent(event);
+
+            // This is the same as DomElement.fireEvent() and was copied
+            // here so it could be used with HtmlPage.
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Firing " + event);
+            }
+            final EventTarget jsNode = this.getScriptableObject();
+            final ContextFactory cf = ((JavaScriptEngine) getWebClient().getJavaScriptEngine()).getContextFactory();
+            final ScriptResult result = cf.call(cx -> jsNode.fireEvent(event));
+
             if (!isOnbeforeunloadAccepted(this, event, result)) {
                 return false;
             }
@@ -1245,7 +1251,12 @@
                     else {
                         event = new Event(frame, eventType);
                     }
-                    final ScriptResult result = ((Node) frame.getScriptableObject()).executeEventLocally(event);
+                    // This fires the "load" event for the <frame> element which, like all non-window
+                    // load events, propagates up to Document but not Window.  The "load" event for
+                    // <frameset> on the other hand, like that of <body>, is handled above where it is
+                    // fired against Document and directed to Window.
+                    final ScriptResult result = frame.fireEvent(event);
+
                     if (!isOnbeforeunloadAccepted((HtmlPage) frame.getPage(), event, result)) {
                         return false;
                     }

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-12 12:32:23 UTC (rev 15516)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/EventListenersContainer.java	2018-08-14 19:17:44 UTC (rev 15517)
@@ -19,10 +19,10 @@
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
-import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -29,7 +29,6 @@
 
 import com.gargoylesoftware.htmlunit.ScriptResult;
 import com.gargoylesoftware.htmlunit.html.DomNode;
-import com.gargoylesoftware.htmlunit.html.HtmlBody;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
 import com.gargoylesoftware.htmlunit.javascript.host.Window;
 import com.gargoylesoftware.htmlunit.javascript.host.html.HTMLDocument;
@@ -54,35 +53,81 @@
 
     private static final Log LOG = LogFactory.getLog(EventListenersContainer.class);
 
-    static class TypeContainer implements Serializable {
-        private List<Scriptable> capturingListeners_;
-        private List<Scriptable> bubblingListeners_;
-        private Function handler_;
+    private static class TypeContainer implements Serializable {
+        public static final TypeContainer EMPTY = new TypeContainer();
 
+        // This sentinel value could be some singleton instance but null
+        // isn't used for anything else so why not.
+        private static final Scriptable EVENT_HANDLER_PLACEHOLDER = null;
+
+        private final List<Scriptable> capturingListeners_;
+        private final List<Scriptable> bubblingListeners_;
+        private final List<Scriptable> atTargetListeners_;
+        private final Function handler_;
+
         TypeContainer() {
-            capturingListeners_ = Collections.unmodifiableList(new ArrayList<Scriptable>());
-            bubblingListeners_ = Collections.unmodifiableList(new ArrayList<Scriptable>());
+            capturingListeners_ = Collections.emptyList();
+            bubblingListeners_ = Collections.emptyList();
+            atTargetListeners_ = Collections.emptyList();
+            handler_ = null;
         }
 
         private TypeContainer(final List<Scriptable> capturingListeners,
-                    final List<Scriptable> bubblingListeners, final Function handler) {
-            capturingListeners_ = Collections.unmodifiableList(new ArrayList<>(capturingListeners));
-            bubblingListeners_ = Collections.unmodifiableList(new ArrayList<>(bubblingListeners));
+                    final List<Scriptable> bubblingListeners, final List<Scriptable> atTargetListeners,
+                    final Function handler) {
+            capturingListeners_ = capturingListeners;
+            bubblingListeners_ = bubblingListeners;
+            atTargetListeners_ = atTargetListeners;
             handler_ = handler;
         }
 
-        private List<Scriptable> getListeners(final boolean useCapture) {
-            if (useCapture) {
-                return capturingListeners_;
+        private List<Scriptable> getListeners(final int eventPhase) {
+            switch (eventPhase) {
+                case Event.CAPTURING_PHASE:
+                    return capturingListeners_;
+                case Event.AT_TARGET:
+                    return atTargetListeners_;
+                case Event.BUBBLING_PHASE:
+                    return bubblingListeners_;
+                default:
+                    throw new UnsupportedOperationException("eventPhase: " + eventPhase);
             }
-            return bubblingListeners_;
         }
 
-        private synchronized boolean addListener(final Scriptable listener, final boolean useCapture) {
-            final List<Scriptable> listeners = getListeners(useCapture);
+        public TypeContainer setPropertyHandler(final Function propertyHandler) {
+            if (propertyHandler != null) {
+                // If we already have a handler then the position of the existing
+                // placeholder should not be changed so just change the handler
+                if (handler_ != null) {
+                    if (propertyHandler == handler_) {
+                        return this;
+                    }
+                    return withPropertyHandler(propertyHandler);
+                }
 
+                // Insert the placeholder and set the handler
+                return withPropertyHandler(propertyHandler).addListener(EVENT_HANDLER_PLACEHOLDER, false);
+            }
+            else {
+                if (handler_ == null) {
+                    return this;
+                }
+                return removeListener(EVENT_HANDLER_PLACEHOLDER, false).withPropertyHandler(null);
+            }
+        }
+
+        private TypeContainer withPropertyHandler(final Function propertyHandler) {
+            return new TypeContainer(capturingListeners_, bubblingListeners_, atTargetListeners_, propertyHandler);
+        }
+
+        public TypeContainer addListener(final Scriptable listener, final boolean useCapture) {
+
+            List<Scriptable> capturingListeners = capturingListeners_;
+            List<Scriptable> bubblingListeners = bubblingListeners_;
+            final List<Scriptable> listeners = useCapture ? capturingListeners : bubblingListeners;
+
             if (listeners.contains(listener)) {
-                return false;
+                return this;
             }
 
             List<Scriptable> newListeners = new ArrayList<>(listeners.size() + 1);
@@ -91,21 +136,29 @@
             newListeners = Collections.unmodifiableList(newListeners);
 
             if (useCapture) {
-                capturingListeners_ = newListeners;
+                capturingListeners = newListeners;
             }
             else {
-                bubblingListeners_ = newListeners;
+                bubblingListeners = newListeners;
             }
 
-            return true;
+            List<Scriptable> atTargetListeners = new ArrayList<>(atTargetListeners_.size() + 1);
+            atTargetListeners.addAll(atTargetListeners_);
+            atTargetListeners.add(listener);
+            atTargetListeners = Collections.unmodifiableList(atTargetListeners);
+
+            return new TypeContainer(capturingListeners, bubblingListeners, atTargetListeners, handler_);
         }
 
-        private synchronized void removeListener(final Scriptable listener, final boolean useCapture) {
-            final List<Scriptable> listeners = getListeners(useCapture);
+        public TypeContainer removeListener(final Scriptable listener, final boolean useCapture) {
 
+            List<Scriptable> capturingListeners = capturingListeners_;
+            List<Scriptable> bubblingListeners = bubblingListeners_;
+            final List<Scriptable> listeners = useCapture ? capturingListeners : bubblingListeners;
+
             final int idx = listeners.indexOf(listener);
             if (idx < 0) {
-                return;
+                return this;
             }
 
             List<Scriptable> newListeners = new ArrayList<>(listeners);
@@ -113,20 +166,32 @@
             newListeners = Collections.unmodifiableList(newListeners);
 
             if (useCapture) {
-                capturingListeners_ = newListeners;
+                capturingListeners = newListeners;
             }
             else {
-                bubblingListeners_ = newListeners;
+                bubblingListeners = newListeners;
             }
+
+            List<Scriptable> atTargetListeners = new ArrayList<>(atTargetListeners_);
+            atTargetListeners.remove(listener);
+            atTargetListeners = Collections.unmodifiableList(atTargetListeners);
+
+            return new TypeContainer(capturingListeners, bubblingListeners, atTargetListeners, handler_);
         }
 
+        // Refactoring note: This method doesn't appear to be used
         @Override
         protected TypeContainer clone() {
-            return new TypeContainer(capturingListeners_, bubblingListeners_, handler_);
+            return new TypeContainer(capturingListeners_, bubblingListeners_, atTargetListeners_, handler_);
         }
     }
 
-    private final Map<String, TypeContainer> typeContainers_ = new HashMap<>();
+    // Refactoring note: This seems ad-hoc..  Shouldn't synchronization be orchestrated between
+    // JS thread and main thread at a much higher layer?  Anyways, to preserve behaviour of prior
+    // coding where 'synchronized' was used more explicitly, we're using a ConcurrentHashMap here
+    // and using ConcurrentMap.compute() to mutate below so that mutations are atomic.  This for
+    // example avoids the case where two concurrent addListener()s can result in either being lost.
+    private final ConcurrentMap<String, TypeContainer> typeContainers_ = new ConcurrentHashMap<>();
     private final EventTarget jsNode_;
 
     /**
@@ -151,9 +216,17 @@
             return true;
         }
 
-        final TypeContainer container = getTypeContainer(type);
-        final boolean added = container.addListener(listener, useCapture);
-        if (!added) {
+        final boolean[] added = {false};
+        typeContainers_.compute(type.toLowerCase(Locale.ROOT), (k, container) -> {
+            if (container == null) {
+                container = TypeContainer.EMPTY;
+            }
+            final TypeContainer newContainer = container.addListener(listener, useCapture);
+            added[0] = newContainer != container;
+            return newContainer;
+        });
+
+        if (!added[0]) {
             if (LOG.isDebugEnabled()) {
                 LOG.debug(type + " listener already registered, skipping it (" + listener + ")");
             }
@@ -164,7 +237,7 @@
 
     private TypeContainer getTypeContainer(final String type) {
         final String typeLC = type.toLowerCase(Locale.ROOT);
-        return typeContainers_.computeIfAbsent(typeLC, k -> new TypeContainer());
+        return typeContainers_.getOrDefault(typeLC, TypeContainer.EMPTY);
     }
 
     /**
@@ -172,14 +245,10 @@
      *
      * @param eventType the event type
      * @param useCapture whether to use capture of not
-     * @return the listeners list
+     * @return the listeners list (empty list when empty)
      */
     public List<Scriptable> getListeners(final String eventType, final boolean useCapture) {
-        final TypeContainer container = typeContainers_.get(eventType.toLowerCase(Locale.ROOT));
-        if (container != null) {
-            return container.getListeners(useCapture);
-        }
-        return null;
+        return getTypeContainer(eventType).getListeners(useCapture ? Event.CAPTURING_PHASE : Event.BUBBLING_PHASE);
     }
 
     /**
@@ -194,10 +263,8 @@
             return;
         }
 
-        final TypeContainer container = typeContainers_.get(eventType.toLowerCase(Locale.ROOT));
-        if (container != null) {
-            container.removeListener(listener, useCapture);
-        }
+        typeContainers_.computeIfPresent(eventType.toLowerCase(Locale.ROOT),
+            (k, container) -> container.removeListener(listener, useCapture));
     }
 
     /**
@@ -216,11 +283,15 @@
             handler = (Function) value;
         }
 
-        final TypeContainer container = getTypeContainer(eventType);
-        container.handler_ = handler;
+        typeContainers_.compute(eventType.toLowerCase(Locale.ROOT), (k, container) -> {
+            if (container == null) {
+                container = TypeContainer.EMPTY;
+            }
+            return container.setPropertyHandler(handler);
+        });
     }
 
-    private ScriptResult executeEventListeners(final boolean useCapture, final Event event, final Object[] args) {
+    private ScriptResult executeEventListeners(final int eventPhase, final Event event, final Object[] args) {
         final DomNode node = jsNode_.getDomNodeOrNull();
         // some event don't apply on all kind of nodes, for instance "blur"
         if (node != null && !node.handles(event)) {
@@ -228,8 +299,9 @@
         }
 
         ScriptResult allResult = null;
-        final List<Scriptable> listeners = getListeners(event.getType(), useCapture);
-        if (listeners != null && !listeners.isEmpty()) {
+        final TypeContainer container = getTypeContainer(event.getType());
+        final List<Scriptable> listeners = container.getListeners(eventPhase);
+        if (!listeners.isEmpty()) {
             event.setCurrentTarget(jsNode_);
 
             final HtmlPage page;
@@ -250,7 +322,12 @@
             }
 
             // no need for a copy, listeners are copy on write
-            for (final Scriptable listener : listeners) {
+            for (Scriptable listener : listeners) {
+                boolean isPropertyHandler = false;
+                if (listener == TypeContainer.EVENT_HANDLER_PLACEHOLDER) {
+                    listener = container.handler_;
+                    isPropertyHandler = true;
+                }
                 Function function = null;
                 Scriptable thisObject = null;
                 if (listener instanceof Function) {
@@ -267,7 +344,8 @@
                 if (function != null) {
                     final ScriptResult result =
                             page.executeJavaScriptFunction(function, thisObject, args, node);
-                    if (event.isPropagationStopped()) {
+                    // Return value is only honoured for property handlers (Chrome/FF)
+                    if (isPropertyHandler) {
                         allResult = result;
                     }
                     if (jsNode_.getBrowserVersion().hasFeature(EVENT_FALSE_RESULT)) {
@@ -290,53 +368,14 @@
         return allResult;
     }
 
-    private ScriptResult executeEventHandler(final Event event, final Object[] propHandlerArgs) {
-        final DomNode node = jsNode_.getDomNodeOrNull();
-        // some event don't apply on all kind of nodes, for instance "blur"
-        if (node != null && !node.handles(event)) {
-            return null;
-        }
-        final Function handler = getEventHandler(event.getType());
-        if (handler != null) {
-            event.setCurrentTarget(jsNode_);
-            final HtmlPage page = (HtmlPage) (node != null
-                    ? node.getPage()
-                    : jsNode_.getWindow().getWebWindow().getEnclosedPage());
-            if (LOG.isDebugEnabled()) {
-                LOG.debug("Executing " + event.getType() + " handler for " + node);
-            }
-            return page.executeJavaScriptFunction(handler, jsNode_,
-                    propHandlerArgs, page);
-        }
-        return null;
-    }
-
     /**
      * Executes bubbling listeners.
      * @param event the event
      * @param args arguments
-     * @param propHandlerArgs handler arguments
      * @return the result
      */
-    public ScriptResult executeBubblingListeners(final Event event, final Object[] args,
-            final Object[] propHandlerArgs) {
-        ScriptResult result = null;
-
-        // the handler declared as property if any (not on body, as handler declared on body goes to the window)
-        final DomNode domNode = jsNode_.getDomNodeOrNull();
-        if (!(domNode instanceof HtmlBody)) {
-            result = executeEventHandler(event, propHandlerArgs);
-            if (event.isPropagationStopped()) {
-                return result;
-            }
-        }
-
-        // the registered listeners (if any)
-        final ScriptResult newResult = executeEventListeners(false, event, args);
-        if (newResult != null) {
-            result = newResult;
-        }
-        return result;
+    public ScriptResult executeBubblingListeners(final Event event, final Object[] args) {
+        return executeEventListeners(Event.BUBBLING_PHASE, event, args);
     }
 
     /**
@@ -346,20 +385,26 @@
      * @return the result
      */
     public ScriptResult executeCapturingListeners(final Event event, final Object[] args) {
-        return executeEventListeners(true, event, args);
+        return executeEventListeners(Event.CAPTURING_PHASE, event, args);
     }
 
     /**
+     * Executes listeners for events targeting the node. (non-propagation phase)
+     * @param event the event
+     * @param args the arguments
+     * @return the result
+     */
+    public ScriptResult executeAtTargetListeners(final Event event, final Object[] args) {
+        return executeEventListeners(Event.AT_TARGET, event, args);
+    }
+
+    /**
      * Returns an event handler.
      * @param eventType the event name (e.g. "click")
      * @return the handler function, {@code null} if the property is null or not a function
      */
     public Function getEventHandler(final String eventType) {
-        final TypeContainer container = typeContainers_.get(eventType.toLowerCase(Locale.ROOT));
-        if (container == null) {
-            return null;
-        }
-        return (Function) container.handler_;
+        return getTypeContainer(eventType).handler_;
     }
 
     /**
@@ -368,50 +413,10 @@
      * @return {@code true} if there are any event listeners for the specified event, {@code false} otherwise
      */
     boolean hasEventListeners(final String eventType) {
-        final TypeContainer container = typeContainers_.get(eventType);
-        return container != null
-            && (container.handler_ instanceof Function
-                    || !container.bubblingListeners_.isEmpty()
-                    || !container.capturingListeners_.isEmpty());
+        return !getTypeContainer(eventType).atTargetListeners_.isEmpty();
     }
 
     /**
-     * Executes listeners.
-     *
-     * @param event the event
-     * @param args the arguments
-     * @param propHandlerArgs handler arguments
-     * @return the result
-     */
-    ScriptResult executeListeners(final Event event, final Object[] args, final Object[] propHandlerArgs) {
-        // the registered capturing listeners (if any)
-        event.setEventPhase(Event.CAPTURING_PHASE);
-        ScriptResult result = executeEventListeners(true, event, args);
-        if (event.isPropagationStopped()) {
-            return result;
-        }
-
-        // the handler declared as property (if any)
-        event.setEventPhase(Event.AT_TARGET);
-        ScriptResult newResult = executeEventHandler(event, propHandlerArgs);
-        if (newResult != null) {
-            result = newResult;
-        }
-        if (event.isPropagationStopped()) {
-            return result;
-        }
-
-        // the registered bubbling listeners (if any)
-        event.setEventPhase(Event.BUBBLING_PHASE);
-        newResult = executeEventListeners(false, event, args);
-        if (newResult != null) {
-            result = newResult;
-        }
-
-        return result;
-    }
-
-    /**
      * {@inheritDoc}
      */
     @Override

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-12 12:32:23 UTC (rev 15516)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/event/EventTarget.java	2018-08-14 19:17:44 UTC (rev 15517)
@@ -40,7 +40,6 @@
 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.html.HTMLLabelElement;
 
 import net.sourceforge.htmlunit.corejs.javascript.Context;
 import net.sourceforge.htmlunit.corejs.javascript.Function;
@@ -98,13 +97,11 @@
         final Window window = getWindow();
         final Object[] args = new Object[] {event};
 
-        // handlers declared as property on a node don't receive the event as argument for IE
-        final Object[] propHandlerArgs = args;
-
         final Event previousEvent = window.getCurrentEvent();
         window.setCurrentEvent(event);
         try {
-            return eventListenersContainer.executeListeners(event, args, propHandlerArgs);
+            event.setEventPhase(Event.AT_TARGET);
+            return eventListenersContainer.executeAtTargetListeners(event, args);
         }
         finally {
             window.setCurrentEvent(previousEvent); // reset event
@@ -125,16 +122,16 @@
         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 {
-            // window's listeners
-            final EventListenersContainer windowsListeners = window.getEventListenersContainer();
+            // 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;
 
-            // capturing phase
-            event.setEventPhase(Event.CAPTURING_PHASE);
-            final boolean windowEventIfDetached = getBrowserVersion().hasFeature(JS_EVENT_WINDOW_EXECUTE_IF_DITACHED);
-
             boolean isAttached = false;
-            for (DomNode node = getDomNodeOrNull(); node != null; node = node.getParentNode()) {
+            for (DomNode node = ourNode; node != null; node = node.getParentNode()) {
                 if (node instanceof Document || node instanceof DomDocumentFragment) {
                     isAttached = true;
                     break;
@@ -141,68 +138,99 @@
                 }
             }
 
-            if (isAttached || windowEventIfDetached) {
-                result = windowsListeners.executeCapturingListeners(event, args);
-                if (event.isPropagationStopped()) {
-                    return result;
+            // 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);
                 }
+                // 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);
+                    }
+                }
             }
-            final List<EventTarget> eventTargetList = new ArrayList<>();
-            EventTarget eventTarget = this;
-            while (eventTarget != null) {
-                if (isAttached) {
-                    eventTargetList.add(eventTarget);
+            // 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)) {
+                    propagationPath.add(window);
                 }
-                final DomNode domNode = eventTarget.getDomNodeOrNull();
-                eventTarget = null;
-                if (domNode != null && domNode.getParentNode() != null) {
-                    eventTarget = domNode.getParentNode().getScriptableObject();
-                }
             }
 
             final boolean ie = getBrowserVersion().hasFeature(JS_CALL_RESULT_IS_LAST_RETURN_VALUE);
-            for (int i = eventTargetList.size() - 1; i >= 0; i--) {
-                final EventTarget jsNode = eventTargetList.get(i);
-                final EventListenersContainer elc = jsNode.eventListenersContainer_;
-                if (elc != null && isAttached) {
-                    final ScriptResult r = elc.executeCapturingListeners(event, args);
-                    result = ScriptResult.combine(r, result, ie);
-                    if (event.isPropagationStopped()) {
-                        return result;
+
+            // 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);
+
+                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;
+                        }
                     }
                 }
             }
 
-            // handlers declared as property on a node don't receive the event as argument for IE
-            final Object[] propHandlerArgs = args;
-
-            // bubbling phase
+            // at target phase
             event.setEventPhase(Event.AT_TARGET);
-            eventTarget = this;
-            HtmlLabel label = null;
-            final boolean processLabelAfterBubbling = event.processLabelAfterBubbling();
 
-            while (eventTarget != null) {
-                final EventTarget jsNode = eventTarget;
+            if (!propagationPath.isEmpty()) {
+                // Note: This element is not always the same as event.getTarget():
+                // e.g. the 'load' event targets Document but "at target" is on Window.
+                final EventTarget jsNode = propagationPath.get(0);
                 final EventListenersContainer elc = jsNode.eventListenersContainer_;
-                if (elc != null && !(jsNode instanceof Window) && (isAttached || !(jsNode instanceof HTMLElement))) {
-                    final ScriptResult r = elc.executeBubblingListeners(event, args, propHandlerArgs);
+                if (elc != null) {
+                    final ScriptResult r = elc.executeAtTargetListeners(event, args);
                     result = ScriptResult.combine(r, result, ie);
                     if (event.isPropagationStopped()) {
                         return result;
                     }
                 }
-                final DomNode domNode = eventTarget.getDomNodeOrNull();
-                eventTarget = null;
-                if (domNode != null && domNode.getParentNode() != null) {
-                    eventTarget = domNode.getParentNode().getScriptableObject();
+            }
+
+            // Refactoring note: This should probably be done further down
+            HtmlLabel label = null;
+            if (event.processLabelAfterBubbling()) {
+                for (DomNode parent = ourParentNode; parent != null; parent = parent.getParentNode()) {
+                    if (parent instanceof HtmlLabel) {
+                        label = (HtmlLabel) parent;
+                        break;
+                    }
                 }
+            }
+
+            // bubbling phase
+            if (event.isBubbles()) {
+                // This belongs here inside the block because events that don't bubble never set
+                // eventPhase = 3 (tested in Chrome)
                 event.setEventPhase(Event.BUBBLING_PHASE);
 
-                if (eventTarget != null
-                        && label == null
-                        && processLabelAfterBubbling && eventTarget instanceof HTMLLabelElement) {
-                    label = (HtmlLabel) eventTarget.getDomNodeOrNull();
+                for (int i = 1, size = propagationPath.size(); i < size; i++) {
+                    final EventTarget jsNode = propagationPath.get(i);
+                    final EventListenersContainer elc = jsNode.eventListenersContainer_;
+                    if (elc != null) {
+                        final ScriptResult r = elc.executeBubblingListeners(event, args);
+                        result = ScriptResult.combine(r, result, ie);
+                        if (event.isPropagationStopped()) {
+                            return result;
+                        }
+                    }
                 }
             }
 
@@ -218,10 +246,6 @@
                 }
             }
 
-            if (isAttached || windowEventIfDetached) {
-                final ScriptResult r = windowsListeners.executeBubblingListeners(event, args, propHandlerArgs);
-                result = ScriptResult.combine(r, result, ie);
-            }
         }
         finally {
             event.endFire();

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-12 12:32:23 UTC (rev 15516)
+++ trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/Window3Test.java	2018-08-14 19:17:44 UTC (rev 15517)
@@ -16,9 +16,13 @@
 
 import static com.gargoylesoftware.htmlunit.BrowserRunner.TestedBrowser.IE;
 
+import java.io.InputStream;
 import java.net.URL;
+import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 
+import org.apache.commons.io.IOUtils;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.openqa.selenium.By;
@@ -31,6 +35,7 @@
 import com.gargoylesoftware.htmlunit.BrowserRunner.NotYetImplemented;
 import com.gargoylesoftware.htmlunit.WebDriverTestCase;
 import com.gargoylesoftware.htmlunit.html.HtmlPageTest;
+import com.gargoylesoftware.htmlunit.util.NameValuePair;
 
 /**
  * Tests for {@link Window}.
@@ -1572,4 +1577,574 @@
 
         loadPageWithAlerts2(html);
     }
+
+    /**
+     * Tests the ordering of DOMContentLoaded for window and document
+     * as well as how capturing / bubbling phases are handled.
+     * Tests the ordering of load for window and document, and how they
+     * relate to the onload property of 'body'.
+     * Verifies handling of the at target phase.
+     * Checks the state of event.eventPhase for a non-bubbling event after the bubbling phase.
+     *
+     * @throws Exception if the test fails
+     */
+    @Test
+    @Alerts(DEFAULT = {"window DOMContentLoaded 1 capture",
+                        "window DOMContentLoaded 2 capture",
+                        "document DOMContentLoaded 1",
+                        "document DOMContentLoaded 1 capture",
+                        "document DOMContentLoaded 2",
+                        "document DOMContentLoaded 2 capture",
+                        "window DOMContentLoaded 1",
+                        "window DOMContentLoaded 2",
+                        "window at load 1",
+                        "window at load 1 capture",
+                        "window at load 2",
+                        "onload 2",
+                        "window at load 2 capture",
+                        "after"},
+            IE = {"window DOMContentLoaded 1 capture",
+                        "window DOMContentLoaded 2 capture",
+                        "document DOMContentLoaded 1",
+                        "document DOMContentLoaded 1 capture",
+                        "document DOMContentLoaded 2",
+                        "document DOMContentLoaded 2 capture",
+                        "window DOMContentLoaded 1",
+                        "window DOMContentLoaded 2",
+                        "window at load 1",
+                        "window at load 1 capture",
+                        "window at load 2",
+                        "onload 2",
+                        "window at load 2 capture",
+                        "document at load 1 capture",
+                        "document at load 2 capture",
+                        "document at load 1 capture",
+                        "document at load 2 capture",
+                        "after"})
+    public void onload() 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"
+
+            // These 'load' events and 'onload' property below target 'document' when fired
+            // but path 'window' only. (Chrome/FF)
+            // This is unlike other events where the path always includes the target and
+            // all ancestors up to 'window'. Ascertaining this is possible by inspecting
+            // the 'event' object which is a property of 'window' in Chrome, or
+            // obtained via the first parameter of the event function in FF: e.g. function (event) { log('xyz', event) }
+            + "  window.addEventListener('load', function () { log('window at load 1') })\n"
+
+            // This 'onload' callback is called when the 'load' event is fired.
+            // Ordering of the call is preserved with respect to other 'load' callbacks and is relative to
+            // the position the property is set.  Subsequent overwriting of 'window.onload' with another
+            // valid function does not move this position.  However, setting 'window.onload' to null or a
+            // non-function value will reset the position and a new position us determined the next time
+            // the property is set. The 'body' tag with an 'onload' property behaves synonymously as
+            // writing 'window.onload = function () { ... }'
+            // at the position the 'body' tag appears.
+            //window.onload = function () { log('onload 1') }
+
+            + "  window.addEventListener('load', function () { log('window at load 1 capture') }, true)\n"
+            // This 'DOMContentLoaded' event targets 'document' and paths [window, document] as expected. (Chrome/FF)
+            + "  window.addEventListener('DOMContentLoaded', function () { log('window DOMContentLoaded 1') })\n"
+            + "  window.addEventListener('DOMContentLoaded', "
+                    + "function () { log('window DOMContentLoaded 1 capture') }, true)\n"
+
+            + "  document.addEventListener('load', function () { log('document at load 1') })\n"
+            + "  document.addEventListener('load', function () { log('document at load 1 capture') }, true)\n"
+            + "  document.addEventListener('DOMContentLoaded', function () { log('document DOMContentLoaded 1') })\n"
+            + "  document.addEventListener('DOMContentLoaded', "
+                    + "function () { log('document DOMContentLoaded 1 capture') }, true)\n"
+            + "</script>\n"
+            + "</head>\n"
+            + "<body>\n"
+            + "<script>\n"
+            + "  window.addEventListener('load', function () { log('window at load 2') })\n"
+            //window.onload = null
+            //window.onload = 123
+            + "  window.onload = function () { log('onload 2') }\n"
+            + "  window.addEventListener('load', function () { log('window at load 2 capture') }, true)\n"
+            + "  window.addEventListener('DOMContentLoaded', function () { log('window DOMContentLoaded 2') })\n"
+            + "  window.addEventListener('DOMContentLoaded', "
+                    + "function () { log('window DOMContentLoaded 2 capture') }, true)\n"
+
+            + "  document.addEventListener('load', function () { log('document at load 2 capture') }, true)\n"
+            + "  document.addEventListener('DOMContentLoaded', function () { log('document DOMContentLoaded 2') })\n"
+            + "  document.addEventListener('DOMContentLoaded', "
+                    + "function () { log('document DOMContentLoaded 2 capture') }, true)\n"
+
+            // This is for testing the state of event.eventPhase afterwards
+            + "  window.addEventListener('load', "
+                    + "function (event) { var x = event; "
+                        + "window.setTimeout(function () { log('after', x.eventPhase) }, 100) }, true)\n"
+            + "</script>\n"
+            + "  <textarea id='log' rows=40 cols=80></textarea>\n"
+            + "</body></html>";
+
+        final WebDriver driver = loadPage2(html);
+        Thread.sleep(200);
+        final String text = driver.findElement(By.id("log")).getAttribute("value").trim().replaceAll("\r", "");
+        assertEquals(String.join("\n", getExpectedAlerts()), text);
+    }
+
+    /**
+     * Tests load and error events of 'script' tags.
+     * Checks that they should be using EventTarget.fireEvent()
+     * rather than Event.executeEventLocally().
+     *
+     * @throws Exception if the test fails
+     */
+    @Test
+    @Alerts(DEFAULT = {"document DOMContentLoaded",
+                        "window DOMContentLoaded",
+                        "window at load",
+                        "window at load capture",
+                        "body onload"},
+            IE = {"document DOMContentLoaded",
+                        "window DOMContentLoaded",
+                        "window at load",
+                        "window at load capture",
+                        "document at load capture",
+                        "body onload"})
+    public void onloadScript() throws Exception {
+        getMockWebConnection().setResponse(URL_SECOND, "");
+
+        final String html = HtmlPageTest.STANDARDS_MODE_PREFIX_
+            + "<html><head>\n"
+            + "<script>\n"
+            + "  function log(msg) {\n"
+            + "    document.getElementById('log').value += msg + '\\n';\n"
+            + "  }\n"
+
+            + "  window.addEventListener('load', function () { log('window at load') })\n"
+            + "  window.addEventListener('load', function () { log('window at load capture') }, true)\n"
+            + "  window.addEventListener('error', function () { log('window at error') })\n"
+            + "  window.addEventListener('error', function () { log('window at error capture') }, true)\n"
+            + "  window.addEventListener('DOMContentLoaded', function () { log('window DOMContentLoaded') })\n"
+
+            + "  document.addEventListener('load', function () { log('document at load') })\n"
+            + "  document.addEventListener('load', function () { log('document at load capture') }, true)\n"
+            + "  document.addEventListener('error', function () { log('document at error') })\n"
+            + "  document.addEventListener('error', function () { log('document at error capture') }, true)\n"
+            + "  document.addEventListener('DOMContentLoaded', function () { log('document DOMContentLoaded') })\n"
+
+            + "</script>\n"
+            + "</head>\n"
+            + "<body onload='log(\"body onload\")'>\n"
+            + "  <script src='" + URL_SECOND + "' onload='log(\"element 1 onload\")' "
+                                        + "onerror='log(\"element 1 onerror\")'></script>\n"
+            + "  <script src='missing.txt' onload='log(\"element 2 onload\")' "
+                                        + "onerror='log(\"element 2 onerror\")'></script>\n"
+
+            + "  <textarea id='log' rows=40 cols=80></textarea>\n"
+            + "</body></html>";
+
+        final WebDriver driver = loadPage2(html);
+        Thread.sleep(200);
+        final String text = driver.findElement(By.id("log")).getAttribute("value").trim().replaceAll("\r", "");
+        assertEquals(String.join("\n", getExpectedAlerts()), text);
+    }
+
+    /**
+     * Tests load and error events of 'img' tags.
+     * Checks that they should be using EventTarget.fireEvent()
+     * rather than Event.executeEventLocally().
+     *
+     * @throws Exception if the test fails
+     */
+    @Test
+    @Alerts(DEFAULT = {"window at error capture",
+                        "document at error capture",
+                        "element 2 onerror",
+                        "document DOMContentLoaded",
+                        "window DOMContentLoaded",
+                        "document at load capture",
+                        "element 1 onload",
+                        "window at load",
+                        "window at load capture",
+                        "body onload"},
+            CHROME = {"document DOMContentLoaded",
+                        "window DOMContentLoaded",
+                        "window at error capture",
+                        "document at error capture",
+                        "element 2 onerror",
+                        "document at load capture",
+                        "element 1 onload",
+                        "window at load",
+                        "window at load capture",
+                        "body onload"},
+            IE = {"document at load capture",
+                        "element 1 onload",
+                        "document DOMContentLoaded",
+                        "window DOMContentLoaded",
+                        "window at load",
+                        "window at load capture",
+                        "body onload",
+                        "document at load capture",
+                        "window at error capture",
+                        "document at error capture",
+                        "element 2 onerror"})
+    public void onloadImg() throws Exception {
+        final URL urlImage = new URL(URL_FIRST, "img.jpg");
+        try (InputStream is = getClass().getClassLoader().getResourceAsStream("testfiles/tiny-jpg.img")) {
+            final byte[] directBytes = IOUtils.toByteArray(is);
+            final List<NameValuePair> emptyList = Collections.emptyList();
+            getMockWebConnection().setResponse(urlImage, directBytes, 200, "ok", "image/jpg", emptyList);
+        }
+
+        final String html = HtmlPageTest.STANDARDS_MODE_PREFIX_
+            + "<html><head>\n"
+            + "<script>\n"
+            + "  function log(msg) {\n"
+            + "    document.getElementById('log').value += msg + '\\n';\n"
+            + "  }\n"
+
+            + "  window.addEventListener('load', function () { log('window at load') })\n"
+            + "  window.addEventListener('load', function () { log('window at load capture') }, true)\n"
+            + "  window.addEventListener('error', function () { log('window at error') })\n"
+            + "  window.addEventListener('error', function () { log('window at error capture') }, true)\n"
+            + "  window.addEventListener('DOMContentLoaded', function () { log('window DOMContentLoaded') })\n"
+
+            + "  document.addEventListener('load', function () { log('document at load') })\n"
+            + "  document.addEventListener('load', function () { log('document at load capture') }, true)\n"
+            + "  document.addEventListener('error', function () { log('document at error') })\n"
+            + "  document.addEventListener('error', function () { log('document at error capture') }, true)\n"
+            + "  document.addEventListener('DOMContentLoaded', function () { log('document DOMContentLoaded') })\n"
+
+            + "</script>\n"
+            + "</head>\n"
+            + "<body onload='log(\"body onload\")'>\n"
+            + "  <img src='" + urlImage + "' onload='log(\"element 1 onload\")' "
+                                        + "onerror='log(\"element 1 onerror\")'>\n"
+            + "  <img src='' onload='log(\"element 2 onload\")' "
+                                        + "onerror='log(\"element 2 onerror\")'>\n"
+
+            + "  <textarea id='log' rows=40 cols=80></textarea>\n"
+            + "</body></html>";
+
+        final WebDriver driver = loadPage2(html);
+        Thread.sleep(200);
+        final String text = driver.findElement(By.id("log")).getAttribute("value").trim().replaceAll("\r", "");
+        assertEquals(String.join("\n", getExpectedAlerts()), text);
+    }
+
+    /**
+     * Same as {@link #onload()} but from frame.
+     *
+     * @throws Exception if the test fails
+     */
+    @Test
+    @Alerts(DEFAULT = {"framing window DOMContentLoaded 1 capture",
+                        "framing document DOMContentLoaded 1",
+                        "framing document DOMContentLoaded 1 capture",
+                        "framing window DOMContentLoaded 1",
+                        "window DOMContentLoaded 1 capture",
+                        "window DOMContentLoaded 2 capture",
+                        "document DOMContentLoaded 1",
+                        "document DOMContentLoaded 1 capture",
+                        "document DOMContentLoaded 2",
+                        "document DOMContentLoaded 2 capture",
+                        "window DOMContentLoaded 1",
+                        "window DOMContentLoaded 2",
+                        "window at load 1",
+                        "window at load 1 capture",
+                        "window at load 2",
+                        "onload 2",
+                        "window at load 2 capture",
+                        "framing document at load 1 capture",
+                        "frame onload",
+                        "framing window at load 1",
+                        "framing window at load 1 capture",
+                        "frameset onload",
+                        "after"},
+            IE = {"framing window DOMContentLoaded 1 capture",
+                        "framing document DOMContentLoaded 1",
+                        "framing document DOMContentLoaded 1 capture",
+                        "framing window DOMContentLoaded 1",
+                        "framing document at load 1 capture",
+                        "window DOMContentLoaded 1 capture",
+                        "window DOMContentLoaded 2 capture",
+                        "document DOMContentLoaded 1",
+                        "document DOMContentLoaded 1 capture",
+                        "document DOMContentLoaded 2",
+                        "document DOMContentLoaded 2 capture",
+                        "window DOMContentLoaded 1",
+                        "window DOMContentLoaded 2",
+                        "window at load 1",
+                        "window at load 1 capture",
+                        "window at load 2",
+                        "onload 2",
+                        "window at load 2 capture",
+                        "framing document at load 1 capture",
+                        "frame onload",
+                        "framing window at load 1",
+                        "framing window at load 1 capture",
+                        "frameset onload",
+                        "document at load 1 capture",
+                        "document at load 2 capture",
+                        "document at load 1 capture",
+                        "document at load 2 capture",
+                        "after"})
+    public void onloadFrame() throws Exception {
+
+        final String content = HtmlPageTest.STANDARDS_MODE_PREFIX_
+            + "<html><head>\n"
+            + "<script>\n"
+            + "  function log(msg) {\n"
+            + "    window.parent.document.title += msg + ';';\n"
+            + "  }\n"
+
+            + "  window.addEventListener('load', function () { log('window at load 1') })\n"
+
+            + "  window.addEventListener('load', function () { log('window at load 1 capture') }, true)\n"
+            + "  window.addEventListener('DOMContentLoaded', function () { log('window DOMContentLoaded 1') })\n"
+            + "  window.addEventListener('DOMContentLoaded', "
+                    + "function () { log('window DOMContentLoaded 1 capture') }, true)\n"
+
+            + "  document.addEventListener('load', function () { log('document at load 1') })\n"
+            + "  document.addEventListener('load', function () { log('document at load 1 capture') }, true)\n"
+            + "  document.addEventListener('DOMContentLoaded', function () { log('document DOMContentLoaded 1') })\n"
+            + "  document.addEventListener('DOMContentLoaded', "
+                    + "function () { log('document DOMContentLoaded 1 capture') }, true)\n"
+            + "</script>\n"
+            + "</head>\n"
+            + "<body >\n"
+            + "<script>\n"
+            + "  window.addEventListener('load', function () { log('window at load 2') })\n"
+            + "  window.onload = function () { log('onload 2') }\n"
+            + "  window.addEventListener('load', function () { log('window at load 2 capture') }, true)\n"
+            + "  window.addEventListener('DOMContentLoaded', function () { log('window DOMContentLoaded 2') })\n"
+            + "  window.addEventListener('DOMContentLoaded', "
+                    + "function () { log('window DOMContentLoaded 2 capture') }, true)\n"
+
+            + "  document.addEventListener('load', function () { log('document at load 2 capture') }, true)\n"
+            + "  document.addEventListener('DOMContentLoaded', function () { log('document DOMContentLoaded 2') })\n"
+            + "  document.addEventListener('DOMContentLoaded', "
+                    + "function () { log('document DOMContentLoaded 2 capture') }, true)\n"
+
+            + "  window.addEventListener('load', "
+                    + "function (event) { var x = event; "
+                        + "window.setTimeout(function () { log('after', x.eventPhase) }, 100) }, true)\n"
+            + "</script>\n"
+            + "</body></html>";
+
+        getMockWebConnection().setDefaultResponse(content);
+
+        final String html = HtmlPageTest.STANDARDS_MODE_PREFIX_
+                + "<html><head>\n"
+                + "<script>\n"
+                + "  function log(msg) {\n"
+                + "    window.document.title += msg + ';';\n"
+                + "  }\n"
+
+                + "  window.addEventListener('load', function () { log('framing window at load 1') })\n"
+                + "  window.addEventListener('load', function () { log('framing window at load 1 capture') }, true)\n"
+                + "  window.addEventListener('DOMContentLoaded', "
+                            + "function () { log('framing window DOMContentLoaded 1') })\n"
+                + "  window.addEventListener('DOMContentLoaded', "
+                            + "function () { log('framing window DOMContentLoaded 1 capture') }, true)\n"
+
+                // should not fire because bubbles = false
+                + "  document.addEventListener('load', "
+                            + "function () { log('framing document at load 1') })\n"
+                + "  document.addEventListener('load', "
+                            + "function () { log('framing document at load 1 capture') }, true)\n"
+                + "  document.addEventListener('DOMContentLoaded', "
+                            + "function () { log('framing document DOMContentLoaded 1') })\n"
+                + "  document.addEventListener('DOMContentLoaded', "
+                            + "function () { log('framing document DOMContentLoaded 1 capture') }, true)\n"
+                + "</script>\n"
+                + "</head>\n"
+                + "<frameset onload='log(\"frameset onload\")'>\n"
+                + "<frame src='test_onload.html' onload='log(\"frame onload\")'>\n"
+                + "</frameset>\n"
+                + "</html>";
+
+        final WebDriver driver = loadPage2(html);
+        Thread.sleep(200);
+        final String text = driver.getTitle().trim().replaceAll(";", "\n").trim();
+        assertEquals(String.join("\n", getExpectedAlerts()), text);
+    }
+
+    /**
+     * Tests propagation of a more or less basic event (click event) with regards to
+     * handling of the capturing / bubbling / at target phases.
+     * Tests listener and property handler ordering.
+     *
+     * @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"})
+    public void propagation() 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"
+            + "</script>\n"
+            + "</head>\n"
+            + "<body>\n"
+            + "  <input id='tester' type='button' value='test' onclick='log(\"onclick\")'>\n"
+            + "  <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"
+
+            + "  tester.addEventListener('click', function () { log('i1 at click 1') })\n"
+            + "  tester.addEventListener('click', function () { log('i1 at click 1 capture') }, true)\n"
+            + "  tester.addEventListener('click', function () { log('i1 at click 2') })\n"
+            + "  tester.onclick = function () { log('onclick 2') }\n"
+            + "  tester.addEventListener('click', function () { log('i1 at click 2 capture') }, true)\n"
+            + "</script>\n"
+            + "</body></html>";
+
+        final WebDriver driver = loadPage2(html);
+        driver.findElement(By.id("tester")).click();
+
+        final String text = driver.findElement(By.id("log")).getAttribute("value").trim().replaceAll("\r", "");
+        assertEquals(String.join("\n", getExpectedAlerts()), text);
+    }
+
+    /**
+     * Similar as {@link #propagation()} except with a deeper propagation path.
+     * Check bubbling propagation after modification of the DOM tree by an intermediate listener.
+     *
+     * @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",
+         ...
 
[truncated message content]

[HtmlUnit] [htmlunit:bugs] #1984 Reworking the JS Event listeners implementation

[RBRi <rb...@us...>]

- **status**: open --> accepted
- **assigned_to**: RBRi
- **Comment**:

Will work on this - looks like a great improvement



---

** [bugs:#1984] Reworking the JS Event listeners implementation**

**Status:** accepted
**Group:** Latest SVN
**Created:** Tue Aug 14, 2018 01:38 PM UTC by Atsushi Nakagawa
**Last Updated:** Tue Aug 14, 2018 02:05 PM UTC
**Owner:** RBRi
**Attachments:**

- [EventListenersContainer.java](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventListenersContainer.java) (16.9 kB; application/octet-stream)
- [EventListenersContainer.java-2.32.txt](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventListenersContainer.java-2.32.txt) (15.5 kB; text/plain)
- [EventTarget.java](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventTarget.java) (14.7 kB; application/octet-stream)
- [EventTarget.java-2.32.txt](https://sourceforge.net/p/htmlunit/bugs/1984/attachment/EventTarget.java-2.32.txt) (13.4 kB; text/plain)


### Problem in brief

Thorough testing has revealed event listeners in HtmlUnit are implemented quite liberally in a few places with regards to how they should work.  It's somewhat surprising they're working so well in practice.

This bug report attempts to address the problems and tackle at fixing them.  Scope includes `xyz.addListenerEvent('foo', function () { ... })` listeners and `xyz.onfoo = function () { ... }` property handlers.

##### Problems noticed

* Ordering of listeners called during the *at target* phase should honour the order in which the listeners were added, but does not.
* Ordering of property handlers such as `window.onload` should honour the timing at which the property was set, with respect to other event listeners, but does not.
* Events where `event.bubbles` is `false` should not have a *bubbling* phase, but does.
* The load event for `Window`should only traverse `Window` but traverses `Window, Document` instead.
* The load event for everything else should traverse from `Document` to all levels down to the element in question, but traverses the element in question only.  (Every place using `EventTarget.executeEventLocally()` potentially applies to this and should probably be using the standard `EventTarget.fireEvent()` instead.)
* The load event for the `<frame>` element should be treated like  "everything else" above, but is not.  (On the other hand, the `onload` property for `<body>` and `<frameset>` is correctly tied to `Window`.)
* The propagation path of events should not be affected by changes to the DOM tree by intermediate listeners, and therefore the *bubbling* phase should always traverse the same nodes as the *capturing* phase only in reverse, but is and does not.
* The return value of event listeners should be ignored (really? it looks that way.. maybe it was different in older IE) and only that of the property handler should be used, but is not.

### Test cases

There are five tests which I'm putting in the comments because it's quite long.

* `test_onload.html`
* `test_frame.html`
* `test_click.html`
* `test_nested_click.html`
* `test_event_return_value.html`

These extra two I'm including but haven't fixed yet.  They're limited to the load/error behaviour of `<script>` and `<img>` and require fixing up some implementation in `HtmlScript` and `HtmlImage`.

* `test_script_onload.html`
* `test_img_onload.html`

### Possible fix

The fix spans three files:

* `html/HtmlPage.java`
* `javascript/host/event/EventListenersContainer.java`
* `javascript/host/event/EventTarget.java`

Changes to `HtmlPage` is the diff below. 

The changes for `EventListenersContainer.java` and `EventTarget.java` are rather extensive so I'm attaching the modified files along with their respective base versions suffixed with `-2.32.txt`.

```diff
--- a/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
+++ b/src/main/java/com/gargoylesoftware/htmlunit/html/HtmlPage.java
@@ -86,15 +86,16 @@
 import com.gargoylesoftware.htmlunit.javascript.PostponedAction;
 import com.gargoylesoftware.htmlunit.javascript.SimpleScriptable;
 import com.gargoylesoftware.htmlunit.javascript.host.Window;
-import com.gargoylesoftware.htmlunit.javascript.host.dom.Node;
 import com.gargoylesoftware.htmlunit.javascript.host.event.BeforeUnloadEvent;
 import com.gargoylesoftware.htmlunit.javascript.host.event.Event;
+import com.gargoylesoftware.htmlunit.javascript.host.event.EventTarget;
 import com.gargoylesoftware.htmlunit.javascript.host.html.HTMLDocument;
 import com.gargoylesoftware.htmlunit.protocol.javascript.JavaScriptURLConnection;
 import com.gargoylesoftware.htmlunit.util.EncodingSniffer;
 import com.gargoylesoftware.htmlunit.util.UrlUtils;
 
 import net.sourceforge.htmlunit.corejs.javascript.Context;
+import net.sourceforge.htmlunit.corejs.javascript.ContextFactory;
 import net.sourceforge.htmlunit.corejs.javascript.Function;
 import net.sourceforge.htmlunit.corejs.javascript.Script;
 import net.sourceforge.htmlunit.corejs.javascript.Scriptable;
@@ -1214,18 +1215,28 @@ private boolean executeEventHandlersIfNeeded(final String eventType) {
         // Execute the specified event on the document element.
         final WebWindow window = getEnclosingWindow();
         if (window.getScriptableObject() instanceof Window) {
-            final DomElement element = getDocumentElement();
-            if (element == null) { // happens for instance if document.documentElement has been removed from parent
-                return true;
-            }
+            // We need the 'Document' node for these load events but getDocumentElement() returns
+            // <html> (HtmlHtml) which is one below that.  The 'Document' node is incidentally just
+            // us (HtmlPage).  (Some tidbits at https://www.w3.org/TR/DOM-Level-3-Events/#event-flow)
+            final DomNode node = this;
             final Event event;
             if (eventType.equals(Event.TYPE_BEFORE_UNLOAD)) {
-                event = new BeforeUnloadEvent(element, eventType);
+                event = new BeforeUnloadEvent(node, eventType);
             }
             else {
-                event = new Event(element, eventType);
+                event = new Event(node, eventType);
+            }
+
+            // This is the same as DomElement.fireEvent() and was copied
+            // here so it could be used with HtmlPage.
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Firing " + event);
             }
-            final ScriptResult result = element.fireEvent(event);
+
+            final EventTarget jsNode = node.getScriptableObject();
+            final ContextFactory cf = ((JavaScriptEngine) getWebClient().getJavaScriptEngine()).getContextFactory();
+            final ScriptResult result = cf.call(cx -> jsNode.fireEvent(event));
+
             if (!isOnbeforeunloadAccepted(this, event, result)) {
                 return false;
             }
@@ -1253,7 +1264,12 @@ private boolean executeEventHandlersIfNeeded(final String eventType) {
                     else {
                         event = new Event(frame, eventType);
                     }
-                    final ScriptResult result = ((Node) frame.getScriptableObject()).executeEventLocally(event);
+
+                    // This fires the "load" event for the <frame> element which, like all non-window
+                    // load events, propagates up to Document but not Window.  The "load" event for
+                    // <frameset> on the other hand, like that of <body>, is handled above where it is
+                    // fired against Document and directed to Window.
+                    final ScriptResult result = frame.fireEvent(event);
                     if (!isOnbeforeunloadAccepted((HtmlPage) frame.getPage(), event, result)) {
                         return false;
                     }
```


---

Sent from sourceforge.net because htm...@li... is subscribed to https://sourceforge.net/p/htmlunit/bugs/

To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/htmlunit/admin/bugs/options.  Or, if this is a mailing list, you can unsubscribe from the mailing list.

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

[<rb...@us...>]

Revision: 15516
          http://sourceforge.net/p/htmlunit/code/15516
Author:   rbri
Date:     2018-08-12 12:32:23 +0000 (Sun, 12 Aug 2018)
Log Message:
-----------
method Date.toUTCString() is available in Rhino; remove our own impl

Modified Paths:
--------------
    trunk/htmlunit/src/changes/changes.xml
    trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/DateCustom.java

Modified: trunk/htmlunit/src/changes/changes.xml
===================================================================
--- trunk/htmlunit/src/changes/changes.xml	2018-08-10 18:13:24 UTC (rev 15515)
+++ trunk/htmlunit/src/changes/changes.xml	2018-08-12 12:32:23 UTC (rev 15516)
@@ -8,12 +8,15 @@
 
     <body>
         <release version="2.33" date="xxxx, 2018" description="Bugfixes">
+            <action type="update" dev="rbri">
+                Method Date.toUTCString() is available in Rhino; remove our own impl.
+            </action>
             <action type="fix" dev="rbri" issue="1979" due-to="Atsushi Nakagawa">
                 Fix the order of the windows returned by WebClient.getTopLevelWindows()
                 to be again the same as WebClient.getWebWindows().
             </action>
             <action type="fix" dev="rbri" issue="1980" due-to="Atsushi Nakagawa">
-                Improved support for javascript named function expressions.
+                Improved support for javascript named function expressions (core-js).
             </action>
             <action type="fix" dev="rbri" issue="1982" due-to="Steve Harney">
                 DomNodeIterator no longer traverses uncles of root node.

Modified: trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/DateCustom.java
===================================================================
--- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/DateCustom.java	2018-08-10 18:13:24 UTC (rev 15515)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/DateCustom.java	2018-08-12 12:32:23 UTC (rev 15516)
@@ -19,9 +19,7 @@
 
 import java.util.Date;
 import java.util.Locale;
-import java.util.TimeZone;
 
-import org.apache.commons.lang3.time.DateFormatUtils;
 import org.apache.commons.lang3.time.FastDateFormat;
 
 import com.gargoylesoftware.htmlunit.BrowserVersion;
@@ -39,8 +37,6 @@
  */
 public final class DateCustom {
 
-    private static final TimeZone UTC_TIME_ZONE = TimeZone.getTimeZone("UTC");
-
     private DateCustom() { }
 
     /**
@@ -94,20 +90,6 @@
         return format.format(getDateValue(thisObj));
     }
 
-    /**
-     * Converts a date to a UTC string. Special version for IE
-     * @param context the JavaScript context
-     * @param thisObj the scriptable
-     * @param args the arguments passed into the method
-     * @param function the function
-     * @return converted string
-     */
-    public static String toUTCString(
-            final Context context, final Scriptable thisObj, final Object[] args, final Function function) {
-        final Date date = new Date(getDateValue(thisObj));
-        return DateFormatUtils.format(date, "EEE, d MMM yyyy HH:mm:ss z", UTC_TIME_ZONE, Locale.ENGLISH);
-    }
-
     private static long getDateValue(final Scriptable thisObj) {
         final Date date = (Date) Context.jsToJava(thisObj, Date.class);
         return date.getTime();

[HtmlUnit] [htmlunit:bugs] #1979 WebClient.getTopLevelWindows() wrongly ordered, probably

[RBRi <rb...@us...>]

- **status**: accepted --> closed



---

** [bugs:#1979] WebClient.getTopLevelWindows() wrongly ordered, probably**

**Status:** closed
**Group:** Latest SVN
**Created:** Thu Aug 02, 2018 07:17 AM UTC by Atsushi Nakagawa
**Last Updated:** Fri Aug 10, 2018 06:14 PM UTC
**Owner:** RBRi


### Problem in brief

Prior to around 2.30, `WebClient.getTopLevelWindows()` was FIFO (i.e. oldest window first), and was in line with `WebClient.getWebWindows()` which is also FIFO.

Now, `WebClient.getTopLevelWindows()` is reversed and is the opposite of `WebClient.getWebWindows()`.

This change is probably an inadvertent resulting from [r15202](https://sourceforge.net/p/htmlunit/code/15202/#diff-2).

### Possible fix

Below is the code we're using locally since changing the spec will break code compatibility.

N.B.: r15202 [also changed the order of window close in  `WebClient.close()`](https://sourceforge.net/p/htmlunit/code/15202/tree/trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java#l1880) but I think that might've been for the better so I've left that one as is.

```diff
--- a/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java
+++ b/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java
@@ -1535,7 +1535,9 @@ public boolean containsWebWindow(final WebWindow webWindow) {
      * @see #getWebWindows()
      */
     public List<TopLevelWindow> getTopLevelWindows() {
-        return Collections.unmodifiableList(new ArrayList<>(topLevelWindows_));
+        List<TopLevelWindow> l = new ArrayList<>(topLevelWindows_.size());
+        topLevelWindows_.descendingIterator().forEachRemaining(l::add);
+        return Collections.unmodifiableList(l);
     }

     /**
```




---

Sent from sourceforge.net because htm...@li... is subscribed to https://sourceforge.net/p/htmlunit/bugs/

To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/htmlunit/admin/bugs/options.  Or, if this is a mailing list, you can unsubscribe from the mailing list.

[HtmlUnit] [htmlunit:bugs] #1979 WebClient.getTopLevelWindows() wrongly ordered, probably

[RBRi <rb...@us...>]

Correct, my fault. This is the result of blindly applying suggestions from code inspections and not having a test.

Many thanks


---

** [bugs:#1979] WebClient.getTopLevelWindows() wrongly ordered, probably**

**Status:** accepted
**Group:** Latest SVN
**Created:** Thu Aug 02, 2018 07:17 AM UTC by Atsushi Nakagawa
**Last Updated:** Fri Aug 10, 2018 06:17 AM UTC
**Owner:** RBRi


### Problem in brief

Prior to around 2.30, `WebClient.getTopLevelWindows()` was FIFO (i.e. oldest window first), and was in line with `WebClient.getWebWindows()` which is also FIFO.

Now, `WebClient.getTopLevelWindows()` is reversed and is the opposite of `WebClient.getWebWindows()`.

This change is probably an inadvertent resulting from [r15202](https://sourceforge.net/p/htmlunit/code/15202/#diff-2).

### Possible fix

Below is the code we're using locally since changing the spec will break code compatibility.

N.B.: r15202 [also changed the order of window close in  `WebClient.close()`](https://sourceforge.net/p/htmlunit/code/15202/tree/trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java#l1880) but I think that might've been for the better so I've left that one as is.

```diff
--- a/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java
+++ b/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java
@@ -1535,7 +1535,9 @@ public boolean containsWebWindow(final WebWindow webWindow) {
      * @see #getWebWindows()
      */
     public List<TopLevelWindow> getTopLevelWindows() {
-        return Collections.unmodifiableList(new ArrayList<>(topLevelWindows_));
+        List<TopLevelWindow> l = new ArrayList<>(topLevelWindows_.size());
+        topLevelWindows_.descendingIterator().forEachRemaining(l::add);
+        return Collections.unmodifiableList(l);
     }

     /**
```




---

Sent from sourceforge.net because htm...@li... is subscribed to https://sourceforge.net/p/htmlunit/bugs/

To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/htmlunit/admin/bugs/options.  Or, if this is a mailing list, you can unsubscribe from the mailing list.

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

[<rb...@us...>]

Revision: 15515
          http://sourceforge.net/p/htmlunit/code/15515
Author:   rbri
Date:     2018-08-10 18:13:24 +0000 (Fri, 10 Aug 2018)
Log Message:
-----------
fix the order of the windows returned by WebClient.getTopLevelWindows()
Issue 1979

Modified Paths:
--------------
    trunk/htmlunit/src/changes/changes.xml
    trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java
    trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/WebClientTest.java

Modified: trunk/htmlunit/src/changes/changes.xml
===================================================================
--- trunk/htmlunit/src/changes/changes.xml	2018-08-10 16:51:51 UTC (rev 15514)
+++ trunk/htmlunit/src/changes/changes.xml	2018-08-10 18:13:24 UTC (rev 15515)
@@ -8,6 +8,10 @@
 
     <body>
         <release version="2.33" date="xxxx, 2018" description="Bugfixes">
+            <action type="fix" dev="rbri" issue="1979" due-to="Atsushi Nakagawa">
+                Fix the order of the windows returned by WebClient.getTopLevelWindows()
+                to be again the same as WebClient.getWebWindows().
+            </action>
             <action type="fix" dev="rbri" issue="1980" due-to="Atsushi Nakagawa">
                 Improved support for javascript named function expressions.
             </action>

Modified: trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java
===================================================================
--- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java	2018-08-10 16:51:51 UTC (rev 15514)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java	2018-08-10 18:13:24 UTC (rev 15515)
@@ -37,12 +37,10 @@
 import java.net.URLDecoder;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
-import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.ConcurrentModificationException;
 import java.util.Date;
-import java.util.Deque;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -171,7 +169,8 @@
     private PageCreator pageCreator_ = new DefaultPageCreator();
 
     private final Set<WebWindowListener> webWindowListeners_ = new HashSet<>(5);
-    private final Deque<TopLevelWindow> topLevelWindows_ = new ArrayDeque<>(); // top-level windows
+    private final List<TopLevelWindow> topLevelWindows_ =
+            Collections.synchronizedList(new ArrayList<TopLevelWindow>()); // top-level windows
     private final List<WebWindow> windows_ = Collections.synchronizedList(new ArrayList<WebWindow>()); // all windows
     private transient List<WeakReference<JavaScriptJobManager>> jobManagers_ =
             Collections.synchronizedList(new ArrayList<WeakReference<JavaScriptJobManager>>());
@@ -1502,6 +1501,8 @@
     /**
      * Returns an immutable list of open web windows (whether they are top level windows or not).
      * This is a snapshot; future changes are not reflected by this list.
+     * <p>
+     * The list is ordered by age, the oldest one first.
      *
      * @return an immutable list of open web windows (whether they are top level windows or not)
      * @see #getWebWindowByName(String)
@@ -1528,6 +1529,8 @@
     /**
      * Returns an immutable list of open top level windows.
      * This is a snapshot; future changes are not reflected by this list.
+     * <p>
+     * The list is ordered by age, the oldest one first.
      *
      * @return an immutable list of open top level windows
      * @see #getWebWindowByName(String)
@@ -1816,18 +1819,25 @@
                     if (webClient_.topLevelWindows_.isEmpty()) {
                         // Must always have at least window, and there are no top-level windows left; must create one.
                         final TopLevelWindow newWindow = new TopLevelWindow("", webClient_);
-                        webClient_.topLevelWindows_.push(newWindow);
+                        webClient_.topLevelWindows_.add(newWindow);
                         webClient_.setCurrentWindow(newWindow);
                     }
                     else {
                         // The current window is now the previous top-level window.
-                        webClient_.setCurrentWindow(webClient_.topLevelWindows_.peek());
+                        webClient_.setCurrentWindow(
+                                webClient_.topLevelWindows_.get(webClient_.topLevelWindows_.size() - 1));
                     }
                 }
             }
             else if (window == webClient_.getCurrentWindow()) {
                 // The current window is now the last top-level window.
-                webClient_.setCurrentWindow(webClient_.topLevelWindows_.peek());
+                if (webClient_.topLevelWindows_.isEmpty()) {
+                    webClient_.setCurrentWindow(null);
+                }
+                else {
+                    webClient_.setCurrentWindow(
+                            webClient_.topLevelWindows_.get(webClient_.topLevelWindows_.size() - 1));
+                }
             }
         }
 
@@ -1876,7 +1886,7 @@
             final WebWindow window = event.getWebWindow();
             if (window instanceof TopLevelWindow) {
                 final TopLevelWindow tlw = (TopLevelWindow) window;
-                webClient_.topLevelWindows_.push(tlw);
+                webClient_.topLevelWindows_.add(tlw);
             }
             // Page is not loaded yet, don't set it now as current window.
         }

Modified: trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/WebClientTest.java
===================================================================
--- trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/WebClientTest.java	2018-08-10 16:51:51 UTC (rev 15514)
+++ trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/WebClientTest.java	2018-08-10 18:13:24 UTC (rev 15515)
@@ -2173,28 +2173,59 @@
         conn.setResponse(URL_SECOND, "<html><body></body></html>");
         client.setWebConnection(conn);
 
+        final WebWindow firstWindow = client.getWebWindows().get(0);
+
         assertEquals(1, client.getWebWindows().size());
         assertEquals(1, client.getTopLevelWindows().size());
+        assertEquals(client.getCurrentWindow(), client.getWebWindows().get(0));
+        assertEquals(client.getCurrentWindow(), client.getTopLevelWindows().get(0));
+        assertEquals(firstWindow, client.getWebWindows().get(0));
+        assertEquals(firstWindow, client.getTopLevelWindows().get(0));
 
         client.getPage(URL_FIRST);
 
         assertEquals(2, client.getWebWindows().size());
         assertEquals(1, client.getTopLevelWindows().size());
+        assertEquals(client.getCurrentWindow(), client.getWebWindows().get(0));
+        assertEquals(client.getCurrentWindow(), client.getTopLevelWindows().get(0));
+        assertEquals(firstWindow, client.getWebWindows().get(0));
+        assertEquals(firstWindow, client.getTopLevelWindows().get(0));
 
         client.getPage(URL_SECOND);
 
         assertEquals(1, client.getWebWindows().size());
         assertEquals(1, client.getTopLevelWindows().size());
+        assertEquals(client.getCurrentWindow(), client.getWebWindows().get(0));
+        assertEquals(client.getCurrentWindow(), client.getTopLevelWindows().get(0));
+        assertEquals(firstWindow, client.getWebWindows().get(0));
+        assertEquals(firstWindow, client.getTopLevelWindows().get(0));
 
         client.openWindow(URL_SECOND, "a");
 
         assertEquals(2, client.getWebWindows().size());
         assertEquals(2, client.getTopLevelWindows().size());
+        assertEquals(client.getCurrentWindow(), client.getWebWindows().get(1));
+        assertEquals(client.getCurrentWindow(), client.getTopLevelWindows().get(1));
+        assertEquals(client.getWebWindows().get(1), client.getTopLevelWindows().get(1));
+        assertEquals(firstWindow, client.getWebWindows().get(0));
+        assertEquals(firstWindow, client.getTopLevelWindows().get(0));
+        assertNotEquals(firstWindow, client.getWebWindows().get(1));
+        assertNotEquals(firstWindow, client.getTopLevelWindows().get(1));
 
         client.openWindow(URL_SECOND, "b");
 
         assertEquals(3, client.getWebWindows().size());
         assertEquals(3, client.getTopLevelWindows().size());
+        assertEquals(client.getCurrentWindow(), client.getWebWindows().get(2));
+        assertEquals(client.getCurrentWindow(), client.getTopLevelWindows().get(2));
+        assertEquals(firstWindow, client.getWebWindows().get(0));
+        assertEquals(firstWindow, client.getTopLevelWindows().get(0));
+        assertEquals(client.getWebWindows().get(1), client.getTopLevelWindows().get(1));
+        assertNotEquals(firstWindow, client.getWebWindows().get(1));
+        assertNotEquals(firstWindow, client.getTopLevelWindows().get(1));
+        assertEquals(client.getWebWindows().get(2), client.getTopLevelWindows().get(2));
+        assertNotEquals(firstWindow, client.getWebWindows().get(2));
+        assertNotEquals(firstWindow, client.getTopLevelWindows().get(2));
 
         client.close();
 

[HtmlUnit] SVN: [15514] trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/ javascript/JavaScriptEngine2Test.java

[<rb...@us...>]

Revision: 15514
          http://sourceforge.net/p/htmlunit/code/15514
Author:   rbri
Date:     2018-08-10 16:51:51 +0000 (Fri, 10 Aug 2018)
Log Message:
-----------
switch to the latest core-js made the related tests passing

Modified Paths:
--------------
    trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/JavaScriptEngine2Test.java

Modified: trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/JavaScriptEngine2Test.java
===================================================================
--- trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/JavaScriptEngine2Test.java	2018-08-10 06:12:02 UTC (rev 15513)
+++ trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/javascript/JavaScriptEngine2Test.java	2018-08-10 16:51:51 UTC (rev 15514)
@@ -328,7 +328,8 @@
     @Test
     @BuggyWebDriver
     public void function_object_method() throws Exception {
-        final String html = "<html><head><title>foo</title><script>\n"
+        final String html = "<html><head>\n"
+                + "<script>\n"
                 + "  try {\n"
                 + "    alert('1');\n"
                 + "    function document.onclick() {\n"
@@ -335,10 +336,10 @@
                 + "      alert('hi');\n"
                 + "    }\n"
                 + "    alert('2');\n"
-                + "  } catch(e) {alert(e)}\n"
-                + "</script></head>\n"
+                + "  } catch(e) { alert(e); }\n"
+                + "</script>\n"
+                + "</head>\n"
                 + "<body>\n"
-                + "  <div id='myDiv'>Hello there</div>\n"
                 + "</body></html>";
 
         try {
@@ -721,14 +722,13 @@
      * @throws Exception if the test fails
      */
     @Test
-    @Alerts({"outer abc = 1", "inner abc = function abc() { alert('inner abc = ' + abc); }"})
-    @NotYetImplemented
+    @Alerts({"outer abc = 1", "inner abc = function"})
     public void functionHasNameOfVarStrictMode() throws Exception {
         final String html = "<html><head>\n"
                 + "<script>\n"
                 + "  'use strict';\n"
                 + "  var abc = 1;\n"
-                + "  var foo = function abc() { alert('inner abc = ' + abc); }\n"
+                + "  var foo = function abc() { alert('inner abc = ' + typeof abc); }\n"
                 + "  alert('outer abc = ' + abc);\n"
                 + "  foo()\n"
                 + "</script>\n"
@@ -744,7 +744,6 @@
      */
     @Test
     @Alerts({"a", "b"})
-    @NotYetImplemented
     public void innerFunctionWithSameName() throws Exception {
         final String html = "<html><head>\n"
                 + "<script>\n"
@@ -773,7 +772,6 @@
      */
     @Test
     @Alerts("a")
-    @NotYetImplemented
     public void innerFunctionWithSameNameAsOutsideStrict() throws Exception {
         final String html = "<html><head>\n"
                 + "<script>\n"
@@ -798,15 +796,16 @@
      * @throws Exception if the test fails
      */
     @Test
-    @Alerts({"function func() { alert(func); }", "outer"})
-    @NotYetImplemented
+    @Alerts({"functionfunc(){alert(norm(func));}", "outer"})
     public void secondFunctionWithSameNameStrict() throws Exception {
         final String html = "<html><head>\n"
                 + "<script>\n"
                 + "  'use strict';\n"
+                + "  function norm(foo) { return ('' + foo).replace(/(\\s)/gm,'') }\n"
+
                 + "  function func () { alert('outer'); }\n"
 
-                + "  var x = function func() { alert(func); }\n"
+                + "  var x = function func() { alert(norm(func)); }\n"
 
                 + "  x();\n"
                 + "  func();\n"