On Wed, 4 Dec 2019 14:07:27 GMT, Arun Joseph <ajos...@openjdk.org> wrote:
> The pull request has been updated with additional changes. > > ---------------- > > Added commits: > - 7f6ed5bf: Added test for createAttribute > > Changes: > - all: https://git.openjdk.java.net/jfx/pull/47/files > - new: https://git.openjdk.java.net/jfx/pull/47/files/2bd56c11..7f6ed5bf > > Webrevs: > - full: https://webrevs.openjdk.java.net/jfx/47/webrev.02 > - incr: https://webrevs.openjdk.java.net/jfx/47/webrev.01-02 > > Issue: https://bugs.openjdk.java.net/browse/JDK-8233747 > Stats: 20 lines in 1 file changed: 20 ins; 0 del; 0 mod > Patch: https://git.openjdk.java.net/jfx/pull/47.diff > Fetch: git fetch https://git.openjdk.java.net/jfx pull/47/head:pull/47 I can confirm that this fixes the problem. The test case attached to the bug and the new unit test you added fail (crash) without your fix and throw the expected exception with your fix. I don't quite understand your earlier comment about needing to change the caller when the type is `ExceptionOr<Ref<T>>`. The specific case in question is one where object passed to `raiseDOMError` is of type `ExceptionOr<Ref<Attr>>`, and your change works fine. What am I missing? I added a comment about formatting (missing spaces around a `+` operator) in the test, but you can fix that before you integrate. This will need a second reviewer, so I request @guruhb to review. modules/javafx.web/src/test/java/test/javafx/scene/web/DOMTest.java line 431: > 430: } catch (Throwable ex) { > 431: fail("DOMException expected but instead threw > "+ex.getClass().getName()); > 432: } Minor: there should be spaces surrounding the `+` here. ---------------- Approved by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/47
