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.
Created attachment 79315 [details] Patch
(Added the link to the bug to the ChangeLog)
Comment on attachment 79315 [details] 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.
(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!
Comment on attachment 79315 [details] Patch Committed in r76084 http://trac.webkit.org/changeset/76084
This test crashes on the bots.
Build 23847 (r76085) was the first to show failures: set([u'fast/dom/Window/open-window-empty-url.html'])
Reverted r76084 for reason: Crashes on the bots Committed r76102: <http://trac.webkit.org/changeset/76102>
It's always sad to roll out a test case, but it seems the bug wasn't actually fixed before this test was added. :(
(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?
http://trac.webkit.org/changeset/76084 might have broken Chromium Linux Release
Leopard and Snow Leopard iirc. Sorry for not writing that in my rollout text.
<rdar://problem/8908391>