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

rbri--- via HtmlUnit-develop <[email protected]> Fri, 17 Aug 2018 18:17:34 +0000
Newsgroups gmane.comp.java.htmlunit.devel
Message-ID <[email protected]>
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);


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