Image.onload() must be processed as postponed action documentation and code style

rbri · rbri · commit 24edb67a94aa · 2024-03-09T18:09:01.000+01:00
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
@@ -18,6 +18,9 @@
                 Jetty 9 websocket-client.
             </action>
 
+            <action type="fix" dev="Lai Quang Duong">
+                Image.onload() must be processed as postponed action.
+            </action>
             <action type="fix" dev="RhinoTeam">
                 core-js: [Symbol.species] for Promise fixed'.
             </action>
diff --git a/src/main/java/org/htmlunit/html/DomElement.java b/src/main/java/org/htmlunit/html/DomElement.java
@@ -972,7 +972,8 @@ public <P extends Page> P click(final boolean shiftKey, final boolean ctrlKey, f
 
         // make enclosing window the current one
         final SgmlPage page = getPage();
-        page.getWebClient().setCurrentWindow(page.getEnclosingWindow());
+        final WebClient webClient = page.getWebClient();
+        webClient.setCurrentWindow(page.getEnclosingWindow());
 
         if (!ignoreVisibility) {
             if (!(page instanceof HtmlPage)) {
@@ -1000,27 +1001,27 @@ public <P extends Page> P click(final boolean shiftKey, final boolean ctrlKey, f
                 mouseDown(shiftKey, ctrlKey, altKey, MouseEvent.BUTTON_LEFT);
             }
 
-            final JavaScriptEngine jsEngine = (JavaScriptEngine)page.getWebClient().getJavaScriptEngine();
+            final AbstractJavaScriptEngine<?> jsEngine = webClient.getJavaScriptEngine();
             jsEngine.holdPosponedActions();
             try {
                 if (handleFocus) {
                     // give focus to current element (if possible) or only remove it from previous one
                     DomElement elementToFocus = null;
                     if (this instanceof SubmittableElement
                         || this instanceof HtmlAnchor
-                            && ATTRIBUTE_NOT_DEFINED != ((HtmlAnchor)this).getHrefAttribute()
+                            && ATTRIBUTE_NOT_DEFINED != ((HtmlAnchor) this).getHrefAttribute()
                         || this instanceof HtmlArea
-                            && (ATTRIBUTE_NOT_DEFINED != ((HtmlArea)this).getHrefAttribute()
-                                || getPage().getWebClient().getBrowserVersion().hasFeature(JS_AREA_WITHOUT_HREF_FOCUSABLE))
-                        || this instanceof HtmlElement && ((HtmlElement)this).getTabIndex() != null) {
+                            && (ATTRIBUTE_NOT_DEFINED != ((HtmlArea) this).getHrefAttribute()
+                                || webClient.getBrowserVersion().hasFeature(JS_AREA_WITHOUT_HREF_FOCUSABLE))
+                        || this instanceof HtmlElement && ((HtmlElement) this).getTabIndex() != null) {
                         elementToFocus = this;
                     }
                     else if (this instanceof HtmlOption) {
-                        elementToFocus = ((HtmlOption)this).getEnclosingSelect();
+                        elementToFocus = ((HtmlOption) this).getEnclosingSelect();
                     }
 
                     if (elementToFocus == null) {
-                        ((HtmlPage)page).setFocusedElement(null);
+                        ((HtmlPage) page).setFocusedElement(null);
                     }
                     else {
                         elementToFocus.focus();
@@ -1032,8 +1033,8 @@ else if (this instanceof HtmlOption) {
                 }
 
                 MouseEvent event = null;
-                if (page.getWebClient().isJavaScriptEnabled()) {
-                    final BrowserVersion browser = page.getWebClient().getBrowserVersion();
+                if (webClient.isJavaScriptEnabled()) {
+                    final BrowserVersion browser = webClient.getBrowserVersion();
                     if (browser.hasFeature(EVENT_ONCLICK_USES_POINTEREVENT)) {
                         event = new PointerEvent(getEventTargetElement(), MouseEvent.TYPE_CLICK, shiftKey,
                                 ctrlKey, altKey, MouseEvent.BUTTON_LEFT, 1);
@@ -1048,7 +1049,8 @@ else if (this instanceof HtmlOption) {
                     }
                 }
                 return click(event, shiftKey, ctrlKey, altKey, ignoreVisibility);
-            } finally {
+            }
+            finally {
                 jsEngine.processPostponedActions();
             }
         }
diff --git a/src/main/java/org/htmlunit/html/HtmlImage.java b/src/main/java/org/htmlunit/html/HtmlImage.java
@@ -41,7 +41,7 @@
 import org.htmlunit.WebClient;
 import org.htmlunit.WebRequest;
 import org.htmlunit.WebResponse;
-import org.htmlunit.javascript.JavaScriptEngine;
+import org.htmlunit.javascript.AbstractJavaScriptEngine;
 import org.htmlunit.javascript.PostponedAction;
 import org.htmlunit.javascript.host.dom.Document;
 import org.htmlunit.javascript.host.event.Event;
@@ -313,7 +313,7 @@ public void execute() {
                 htmlPage.addAfterLoadAction(action);
             }
             else {
-                JavaScriptEngine jsEngine = (JavaScriptEngine) client.getJavaScriptEngine();
+                final AbstractJavaScriptEngine<?> jsEngine = client.getJavaScriptEngine();
                 if (jsEngine.isScriptRunning()) {
                     final PostponedAction action = new PostponedAction(getPage(), "HtmlImage.doOnLoad") {
                         @Override
diff --git a/src/test/java/org/htmlunit/html/HtmlImageTest.java b/src/test/java/org/htmlunit/html/HtmlImageTest.java
@@ -317,39 +317,4 @@ public void clickWithCoordinates() throws Exception {
         img.click(2, 7);
         assertEquals(getExpectedAlerts()[0] + getExpectedAlerts()[1], page.getTitleText());
     }
-
-    /**
-     * @throws Exception on test failure
-     */
-    @Test
-    @Alerts({"in-out-Image.onload(0)-", "mousedown-in-out-Image.onload(1)-mouseup-in-out-click-in-out-Image.onload(2)-Image.onload(3)-"})
-    public void onload() throws Exception {
-        final String html =
-                "<html>\n"
-                + "<head>\n"
-                + "<script>\n"
-                + "  function log(msg) { window.document.title += msg + '-';}\n"
-                + "  function test(i) {\n"
-                + "    log('in');\n"
-                + "    var image = new Image();\n"
-                + "    image.onload = function () { log(\"Image.onload(\" + i + \")\") };\n"
-                + "    image.src = '4x7.jpg';\n"
-                + "    log('out');\n"
-                + "  }\n"
-                + "</script>\n"
-                + "</head>\n"
-                + "<body onload=\"test(0)\">\n"
-                + "<button onmousedown=\"log('mousedown'); test(1)\""
-                + "        onmouseup=\"log('mouseup'); test(2)\""
-                + "        onclick=\"log('click'); test(3)\"></button>\n"
-                + "</body>\n"
-                + "</html>\n";
-
-        final HtmlPage page = loadPage(html);
-        assertEquals(getExpectedAlerts()[0], page.getTitleText());
-
-        page.setTitleText("");
-        page.<HtmlButton>getFirstByXPath("//button").click();
-        assertEquals(getExpectedAlerts()[1], page.getTitleText());
-    }
 }
diff --git a/src/test/java/org/htmlunit/javascript/host/html/HTMLImageElement2Test.java b/src/test/java/org/htmlunit/javascript/host/html/HTMLImageElement2Test.java
@@ -14,11 +14,20 @@
  */
 package org.htmlunit.javascript.host.html;
 
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import org.apache.commons.io.IOUtils;
+import org.htmlunit.CollectingAlertHandler;
 import org.htmlunit.MockWebConnection;
 import org.htmlunit.SimpleWebTestCase;
 import org.htmlunit.WebClient;
+import org.htmlunit.html.HtmlPage;
 import org.htmlunit.junit.BrowserRunner;
 import org.htmlunit.util.MimeType;
+import org.htmlunit.util.NameValuePair;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -51,4 +60,103 @@ public void onLoad_notDownloadedWhenJavascriptDisabled() throws Exception {
         loadPageWithAlerts(html);
         assertEquals(URL_FIRST, conn.getLastWebRequest().getUrl());
     }
+
+    /**
+     * Make sure this works without an exception.
+     *
+     * @throws Exception on test failure
+     */
+    @Test
+    public void onload_complex_JavascriptDisabled() throws Exception {
+        try (InputStream is = getClass().getClassLoader().getResourceAsStream("testfiles/tiny-jpg.img")) {
+            final byte[] directBytes = IOUtils.toByteArray(is);
+
+            final List<NameValuePair> emptyList = Collections.emptyList();
+            getMockWebConnection().setResponse(URL_SECOND, directBytes, 200, "ok", MimeType.IMAGE_JPEG, emptyList);
+        }
+
+        final String html =
+                "<html>\n"
+                + "<head>\n"
+                + "<script>\n"
+                + "  function test(i) {\n"
+                + "    alert('in');\n"
+                + "    var image = new Image();\n"
+                + "    image.onload = function () { alert(\"Image.onload(\" + i + \")\") };\n"
+                + "    image.src = '" + URL_SECOND + "';\n"
+                + "    alert('out');\n"
+                + "  }\n"
+                + "</script>\n"
+                + "</head>\n"
+                + "<body onload='test(0)'>\n"
+                + "<button id='myId'"
+                +           "onmousedown=\"alert('mousedown'); test(1)\" "
+                +           "onmouseup=\"alert('mouseup'); test(2)\" "
+                +           "onclick=\"alert('click'); test(3)\"></button>\n"
+                + "</body>\n"
+                + "</html>\n";
+
+        final WebClient client = getWebClientWithMockWebConnection();
+        client.getOptions().setJavaScriptEnabled(false);
+
+        final List<String> collectedAlerts = new ArrayList<>();
+        client.setAlertHandler(new CollectingAlertHandler(collectedAlerts));
+
+        final HtmlPage page = loadPage(html);
+        page.getElementById("myId").click();
+        assertEquals(getExpectedAlerts(), collectedAlerts);
+        assertEquals(URL_FIRST, getMockWebConnection().getLastWebRequest().getUrl());
+    }
+
+    /**
+     * Make sure this works without an exception.
+     *
+     * @throws Exception on test failure
+     */
+    @Test
+    public void onload_complex_JavascriptEngineDisabled() throws Exception {
+        try (InputStream is = getClass().getClassLoader().getResourceAsStream("testfiles/tiny-jpg.img")) {
+            final byte[] directBytes = IOUtils.toByteArray(is);
+
+            final List<NameValuePair> emptyList = Collections.emptyList();
+            getMockWebConnection().setResponse(URL_SECOND, directBytes, 200, "ok", MimeType.IMAGE_JPEG, emptyList);
+        }
+
+        final String html =
+                "<html>\n"
+                + "<head>\n"
+                + "<script>\n"
+                + "  function test(i) {\n"
+                + "    alert('in');\n"
+                + "    var image = new Image();\n"
+                + "    image.onload = function () { alert(\"Image.onload(\" + i + \")\") };\n"
+                + "    image.src = '" + URL_SECOND + "';\n"
+                + "    alert('out');\n"
+                + "  }\n"
+                + "</script>\n"
+                + "</head>\n"
+                + "<body onload='test(0)'>\n"
+                + "<button id='myId'"
+                +           "onmousedown=\"alert('mousedown'); test(1)\" "
+                +           "onmouseup=\"alert('mouseup'); test(2)\" "
+                +           "onclick=\"alert('click'); test(3)\"></button>\n"
+                + "</body>\n"
+                + "</html>\n";
+
+        try (WebClient webClient = new WebClient(getBrowserVersion(), false, null, -1)) {
+            final List<String> collectedAlerts = new ArrayList<>();
+            webClient.setAlertHandler(new CollectingAlertHandler(collectedAlerts));
+
+            final MockWebConnection webConnection = getMockWebConnection();
+            webConnection.setResponse(URL_FIRST, html);
+
+            webClient.setWebConnection(webConnection);
+
+            final HtmlPage page = loadPage(html);
+            page.getElementById("myId").click();
+            assertEquals(getExpectedAlerts(), collectedAlerts);
+            assertEquals(URL_SECOND, webConnection.getLastWebRequest().getUrl());
+        }
+    }
+
 }
