52654 2011-01-18 12:56:48 -0800 Add a test that calling window.open with an empty URL does not trigger an assertion or cause a crash 2022-07-26 10:05:36 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) All All RESOLVED LATER InRadar P2 Normal --- 1 jberlin jberlin abarth ahmad.saleem792 ap aroben bfulgham eric jberlin rniwa webkit.review.bot oldest_to_newest 336025 0 jberlin 2011-01-18 12:56:48 -0800 In http://trac.webkit.org/changeset/76040 I removed some asserts that were getting triggered by calling window.open with an empty URL, but failed to add a test. 336029 1 79315 jberlin 2011-01-18 12:59:29 -0800 Created attachment 79315 Patch 336031 2 jberlin 2011-01-18 13:01:03 -0800 (Added the link to the bug to the ChangeLog) 336038 3 79315 aroben 2011-01-18 13:08:32 -0800 Comment on attachment 79315 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=79315&action=review > LayoutTests/fast/dom/Window/open-window-empty-url.html:1 > +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> Just <!DOCTYPE html> should be enough. That's what HTML5 recommends. > LayoutTests/fast/dom/Window/open-window-empty-url.html:21 > + <script type="text/javascript"> > + if (window.layoutTestController) { > + layoutTestController.dumpAsText(); > + layoutTestController.waitUntilDone(); > + layoutTestController.setCanOpenWindows(); > + } > + var newWindow = window.open(""); > + newWindow.close(); > + document.getElementById("results").innerText = "PASS: Calling window.open() with an empty URL did not trigger any assertions or cause any crashes."; > + if (window.layoutTestController) > + layoutTestController.notifyDone(); > + </script> > +</body> Since the whole test proceeds synchronously (before the page has even finished loading), there's no need for waitUntilDone/notifyDone. 336223 4 jberlin 2011-01-18 17:07:50 -0800 (In reply to comment #3) > (From update of attachment 79315 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=79315&action=review > > > LayoutTests/fast/dom/Window/open-window-empty-url.html:1 > > +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> > > Just <!DOCTYPE html> should be enough. That's what HTML5 recommends. Done. > > > LayoutTests/fast/dom/Window/open-window-empty-url.html:21 > > + <script type="text/javascript"> > > + if (window.layoutTestController) { > > + layoutTestController.dumpAsText(); > > + layoutTestController.waitUntilDone(); > > + layoutTestController.setCanOpenWindows(); > > + } > > + var newWindow = window.open(""); > > + newWindow.close(); > > + document.getElementById("results").innerText = "PASS: Calling window.open() with an empty URL did not trigger any assertions or cause any crashes."; > > + if (window.layoutTestController) > > + layoutTestController.notifyDone(); > > + </script> > > +</body> > > Since the whole test proceeds synchronously (before the page has even finished loading), there's no need for waitUntilDone/notifyDone. Removed. Thanks for the review! 336226 5 79315 jberlin 2011-01-18 17:14:25 -0800 Comment on attachment 79315 Patch Committed in r76084 http://trac.webkit.org/changeset/76084 336335 6 eric 2011-01-18 22:04:48 -0800 This test crashes on the bots. 336336 7 eric 2011-01-18 22:05:11 -0800 Build 23847 (r76085) was the first to show failures: set([u'fast/dom/Window/open-window-empty-url.html']) 336337 8 eric 2011-01-18 22:09:41 -0800 Reverted r76084 for reason: Crashes on the bots Committed r76102: <http://trac.webkit.org/changeset/76102> 336338 9 eric 2011-01-18 22:10:23 -0800 It's always sad to roll out a test case, but it seems the bug wasn't actually fixed before this test was added. :( 336436 10 aroben 2011-01-19 05:09:09 -0800 (In reply to comment #8) > Reverted r76084 for reason: > > Crashes on the bots > > Committed r76102: <http://trac.webkit.org/changeset/76102> Thanks for rolling it out. Which bots was it crashing on? 336461 11 webkit.review.bot 2011-01-19 06:09:00 -0800 http://trac.webkit.org/changeset/76084 might have broken Chromium Linux Release 336640 12 eric 2011-01-19 10:54:20 -0800 Leopard and Snow Leopard iirc. Sorry for not writing that in my rollout text. 339174 13 jberlin 2011-01-24 12:16:44 -0800 <rdar://problem/8908391> 1886795 14 ahmad.saleem792 2022-07-26 09:44:13 -0700 I changed the test case from the patch into JS Fiddle: Link - https://jsfiddle.net/sd65xt31/show It does not trigger any crash in Safari 15.6 on macOS 12.5 and it just triggers "Pop-up Blocker" and then after it shows (after allowing): ___ Test that calling window.open(""); does not trigger any assertions or cause any crashes. PASS: Calling window.open() with an empty URL did not trigger any assertions or cause any crashes. ___ It is same across all other browsers (Chrome Canary 106 and Firefox Nightly 104). I think if we don't need to land this test case, since it works across all browsers - we can close this as "RESOLVED CONFIGURATION CHANGED". Thanks! 1886805 15 rniwa 2022-07-26 10:05:36 -0700 This is about adding debug asserts. At this point, I don't think keeping this 11 year old bug is the way to address this. If we have had a need to add an assertion in this code, we would have done it meanwhile. -> Later. 79315 2011-01-18 12:59:29 -0800 2011-01-18 17:14:25 -0800 Patch addTestFor52654.patch text/plain 2333 jberlin SW5kZXg6IExheW91dFRlc3RzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9D aGFuZ2VMb2cJKHJldmlzaW9uIDc2MDQ3KQorKysgTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMTMgQEAKKzIwMTEtMDEtMTggIEplc3NpZSBCZXJsaW4gIDxq YmVybGluQGFwcGxlLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBBZGQgYSB0ZXN0IHRoYXQgY2FsbGluZyB3aW5kb3cub3BlbiB3aXRoIGFuIGVt cHR5IFVSTCBkb2VzIG5vdCB0cmlnZ2VyIGFuIGFzc2VydGlvbiBvcgorICAgICAgICBjYXVzZSBh IGNyYXNoLgorCisgICAgICAgICogZmFzdC9kb20vV2luZG93L29wZW4td2luZG93LWVtcHR5LXVy bC1leHBlY3RlZC50eHQ6IEFkZGVkLgorICAgICAgICAqIGZhc3QvZG9tL1dpbmRvdy9vcGVuLXdp bmRvdy1lbXB0eS11cmwuaHRtbDogQWRkZWQuCisKIDIwMTEtMDEtMTggIEtyaXRoaWdhc3NyZWUg U2FtYmFtdXJ0aHkgIDxrcml0aGlnYXNzcmVlLnNhbWJhbXVydGh5QG5va2lhLmNvbT4KIAogICAg ICAgICBSZXZpZXdlZCBieSBTaW1vbiBGcmFzZXIuCkluZGV4OiBMYXlvdXRUZXN0cy9mYXN0L2Rv bS9XaW5kb3cvb3Blbi13aW5kb3ctZW1wdHktdXJsLWV4cGVjdGVkLnR4dAo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBMYXlvdXRUZXN0cy9mYXN0L2RvbS9XaW5kb3cvb3Blbi13aW5kb3ctZW1wdHktdXJsLWV4cGVj dGVkLnR4dAkocmV2aXNpb24gMCkKKysrIExheW91dFRlc3RzL2Zhc3QvZG9tL1dpbmRvdy9vcGVu LXdpbmRvdy1lbXB0eS11cmwtZXhwZWN0ZWQudHh0CShyZXZpc2lvbiAwKQpAQCAtMCwwICsxLDMg QEAKK1Rlc3QgdGhhdCBjYWxsaW5nIHdpbmRvdy5vcGVuKCIiKTsgZG9lcyBub3QgdHJpZ2dlciBh bnkgYXNzZXJ0aW9ucyBvciBjYXVzZSBhbnkgY3Jhc2hlcy4KKworUEFTUzogQ2FsbGluZyB3aW5k b3cub3BlbigpIHdpdGggYW4gZW1wdHkgVVJMIGRpZCBub3QgdHJpZ2dlciBhbnkgYXNzZXJ0aW9u cyBvciBjYXVzZSBhbnkgY3Jhc2hlcy4KSW5kZXg6IExheW91dFRlc3RzL2Zhc3QvZG9tL1dpbmRv dy9vcGVuLXdpbmRvdy1lbXB0eS11cmwuaHRtbAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9m YXN0L2RvbS9XaW5kb3cvb3Blbi13aW5kb3ctZW1wdHktdXJsLmh0bWwJKHJldmlzaW9uIDApCisr KyBMYXlvdXRUZXN0cy9mYXN0L2RvbS9XaW5kb3cvb3Blbi13aW5kb3ctZW1wdHktdXJsLmh0bWwJ KHJldmlzaW9uIDApCkBAIC0wLDAgKzEsMjMgQEAKKzwhRE9DVFlQRSBIVE1MIFBVQkxJQyAiLS8v SUVURi8vRFREIEhUTUwvL0VOIj4KKzxodG1sPgorPGhlYWQ+CisgICAgPHRpdGxlPk9wZW4gV2lu ZG93IHdpdGggYW4gZW1wdHkgVVJMPC90aXRsZT4KKzwvaGVhZD4KKzxib2R5PgorICAgIDxwIGlk PSJkZXNjcmlwdGlvbiI+VGVzdCB0aGF0IGNhbGxpbmcgd2luZG93Lm9wZW4oIiIpOyBkb2VzIG5v dCB0cmlnZ2VyIGFueSBhc3NlcnRpb25zIG9yIGNhdXNlIGFueSBjcmFzaGVzLjwvcD4KKyAgICA8 ZGl2IGlkPSJyZXN1bHRzIj48L2Rpdj4KKyAgICA8c2NyaXB0IHR5cGU9InRleHQvamF2YXNjcmlw dCI+CisgICAgICAgIGlmICh3aW5kb3cubGF5b3V0VGVzdENvbnRyb2xsZXIpIHsKKyAgICAgICAg ICAgIGxheW91dFRlc3RDb250cm9sbGVyLmR1bXBBc1RleHQoKTsKKyAgICAgICAgICAgIGxheW91 dFRlc3RDb250cm9sbGVyLndhaXRVbnRpbERvbmUoKTsKKyAgICAgICAgICAgIGxheW91dFRlc3RD b250cm9sbGVyLnNldENhbk9wZW5XaW5kb3dzKCk7CisgICAgICAgIH0KKyAgICAgICAgdmFyIG5l d1dpbmRvdyA9IHdpbmRvdy5vcGVuKCIiKTsKKyAgICAgICAgbmV3V2luZG93LmNsb3NlKCk7Cisg ICAgICAgIGRvY3VtZW50LmdldEVsZW1lbnRCeUlkKCJyZXN1bHRzIikuaW5uZXJUZXh0ID0gIlBB U1M6IENhbGxpbmcgd2luZG93Lm9wZW4oKSB3aXRoIGFuIGVtcHR5IFVSTCBkaWQgbm90IHRyaWdn ZXIgYW55IGFzc2VydGlvbnMgb3IgY2F1c2UgYW55IGNyYXNoZXMuIjsKKyAgICAgICAgaWYgKHdp bmRvdy5sYXlvdXRUZXN0Q29udHJvbGxlcikKKyAgICAgICAgICAgIGxheW91dFRlc3RDb250cm9s bGVyLm5vdGlmeURvbmUoKTsKKyAgICA8L3NjcmlwdD4KKzwvYm9keT4KKzwvaHRtbD4KKwo=