Revert "[dart:html] Throw exception if Window.open opens null window" This reverts commit a356f71b71bc29c9ab69759609afa2e72a6563e4. Reason for revert: This should be handled by throwing an exception when the methods of the returned window are called, not when it is opened. This would be a noisy breaking change that we don't want for 3.1. For now, revert until the change that affects the individual methods is landed. Original change's description: > [dart:html] Throw exception if Window.open opens null window > > Window.open silently allows a null window to be opened, and > issues arise later when users try to use the non-null wrapper. > This CL changes that to throw an exception if the window is null. > This exception can be caught and recovered from. This avoids the > larger breaking change of making this API nullable. > > CoreLibraryReviewExempt: Backend-specific library. > Change-Id: I9a53a477cb370c3bc6bc26b2162ce66c5af166aa > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/306910 > Reviewed-by: Sigmund Cherem <sigmund@google.com> > Commit-Queue: Srujan Gaddam <srujzs@google.com> CoreLibraryReviewExempt: Revert in backend-specific library. Change-Id: I5007b7d7aa608bfc8e5827b5f967af5573d0b758 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309000 Commit-Queue: Srujan Gaddam <srujzs@google.com> Reviewed-by: Sigmund Cherem <sigmund@google.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md index a8e1bfb..522154c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md
@@ -36,14 +36,6 @@ [#51486]: https://github.com/dart-lang/sdk/issues/51486 [#52027]: https://github.com/dart-lang/sdk/issues/52027 -#### `dart:html` - -- **Breaking change to Window.open**: - `Window.open` will now throw an exception that can be caught - (`NullWindowException`) if the opened window is null. Previously, this null - window would be wrapped, and there would be surprising runtime errors when any - member is used on the wrapper. - #### `dart:js_interop` - **Object literal constructors**:
diff --git a/sdk/lib/html/dart2js/html_dart2js.dart b/sdk/lib/html/dart2js/html_dart2js.dart index e8587eb..cc78dfc 100644 --- a/sdk/lib/html/dart2js/html_dart2js.dart +++ b/sdk/lib/html/dart2js/html_dart2js.dart
@@ -32154,18 +32154,17 @@ /** * Opens a new window. * - * Throws a NullWindowException if the opened window is null. - * * ## Other resources * * * [Window.open](https://developer.mozilla.org/en-US/docs/Web/API/Window.open) * from MDN. */ WindowBase open(String url, String name, [String? options]) { - final win = - options == null ? _open2(url, name) : _open3(url, name, options); - if (win == null) throw new NullWindowException(); - return _DOMWindowCrossFrame._createSafe(win); + if (options == null) { + return _DOMWindowCrossFrame._createSafe(_open2(url, name)); + } else { + return _DOMWindowCrossFrame._createSafe(_open3(url, name, options)); + } } // API level getter and setter for Location. @@ -33778,13 +33777,6 @@ ? JS<num>('num', '#.scrollY', this).round() : document.documentElement!.scrollTop; } - -class NullWindowException implements Exception { - @override - String toString() { - return 'Attempted to call Window.open with a null window.'; - } -} // Copyright (c) 2012, the Dart project authors. Please see the AUTHORS file // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. @@ -40025,7 +40017,7 @@ return e; } -_convertDartToNative_EventTarget(e) { +EventTarget? _convertDartToNative_EventTarget(e) { if (e is _DOMWindowCrossFrame) { return e._window; } else { @@ -40124,7 +40116,7 @@ // Private window. Note, this is a window in another frame, so it // cannot be typed as "Window" as its prototype is not patched // properly. Its fields and methods can only be accessed via JavaScript. - final Object _window; + final _window; // Fields. HistoryBase get history =>
diff --git a/tests/lib/html/window_test.dart b/tests/lib/html/window_test.dart index 2a3fc31..ab79e66 100644 --- a/tests/lib/html/window_test.dart +++ b/tests/lib/html/window_test.dart
@@ -11,13 +11,4 @@ expect(window.scrollX, 0); expect(window.scrollY, 0); }); - test('open', () { - window.open('', 'blank'); - try { - // A blank page with no access to the original window (noopener) should - // result in null. - window.open('', 'invalid', 'noopener=true'); - fail('Expected Window.open to throw.'); - } on NullWindowException {} - }); }
diff --git a/tools/dom/src/dart2js_Conversions.dart b/tools/dom/src/dart2js_Conversions.dart index 9641cb5..cb943b4 100644 --- a/tools/dom/src/dart2js_Conversions.dart +++ b/tools/dom/src/dart2js_Conversions.dart
@@ -33,7 +33,7 @@ return e; } -_convertDartToNative_EventTarget(e) { +EventTarget? _convertDartToNative_EventTarget(e) { if (e is _DOMWindowCrossFrame) { return e._window; } else {
diff --git a/tools/dom/src/dart2js_DOMImplementation.dart b/tools/dom/src/dart2js_DOMImplementation.dart index 29ccba8..8016f6e 100644 --- a/tools/dom/src/dart2js_DOMImplementation.dart +++ b/tools/dom/src/dart2js_DOMImplementation.dart
@@ -9,7 +9,7 @@ // Private window. Note, this is a window in another frame, so it // cannot be typed as "Window" as its prototype is not patched // properly. Its fields and methods can only be accessed via JavaScript. - final Object _window; + final _window; // Fields. HistoryBase get history =>
diff --git a/tools/dom/templates/html/impl/impl_Window.darttemplate b/tools/dom/templates/html/impl/impl_Window.darttemplate index d270c55..ddc1d04 100644 --- a/tools/dom/templates/html/impl/impl_Window.darttemplate +++ b/tools/dom/templates/html/impl/impl_Window.darttemplate
@@ -50,18 +50,17 @@ /** * Opens a new window. * - * Throws a NullWindowException if the opened window is null. - * * ## Other resources * * * [Window.open](https://developer.mozilla.org/en-US/docs/Web/API/Window.open) * from MDN. */ WindowBase open(String url, String name, [String$NULLABLE options]) { - final win = - options == null ? _open2(url, name) : _open3(url, name, options); - if (win == null) throw new NullWindowException(); - return _DOMWindowCrossFrame._createSafe(win); + if (options == null) { + return _DOMWindowCrossFrame._createSafe(_open2(url, name)); + } else { + return _DOMWindowCrossFrame._createSafe(_open3(url, name, options)); + } } // API level getter and setter for Location. @@ -255,10 +254,3 @@ JS<num>('num', '#.scrollY', this).round() : document.documentElement$NULLASSERT.scrollTop; } - -class NullWindowException implements Exception { - @override - String toString() { - return 'Attempted to call Window.open with a null window.'; - } -}
diff --git a/tools/dom/web_library_bindings.dart b/tools/dom/web_library_bindings.dart index 0e10f62..a733d1a 100644 --- a/tools/dom/web_library_bindings.dart +++ b/tools/dom/web_library_bindings.dart
@@ -9581,7 +9581,6 @@ 'NoncedElement': {'NoncedElement'}, 'Notification': {'Notification'}, 'NotificationEvent': {'NotificationEvent'}, - 'NullWindowException': {'NullWindowException'}, 'Number': {'SVGNumber'}, 'NumberInputElement': {'NumberInputElement'}, 'NumberList': {'SVGNumberList'},