Should throw for window.open when URL fails to parse

[zcorpan]

See http://software.hixie.ch/utilities/js/live-dom-viewer/saved/4996

Edge and Gecko throw some exception. WebKit and Chromium return null. We typically throw for APIs when URLs fail to parse, it seems reasonable to do that here as well.

cc @lyzadanger

[zcorpan]

Should be "SyntaxError" DOMException for consistency with other APIs.

[annevk]

@cdumez @jeisinger, thoughts?

[cdumez]

I'm willing to give it a try in WebKit if Blink is on board too.

[lyzadanger]

@annevk I'm fine with this—certainly easier to test for!

[jeisinger]

I think we'd like to have some kind of indication of how many sites will be affected by this change before committing to ship it

[zcorpan]

@jeisinger can you (find someone to) add a use counter?

[jeisinger]

yeah, will do

[zcorpan]

Great, thanks. Let me know when it reaches stable. Then we can also check httparchive instances that trigger the use counter (if there are any).

[foolip]

window.open() is probably called from a click event handler, not during the initial load, so I don't think HTTP Archive will help here.

[zcorpan]

Ah right. Hmmm. I suppose use counter + the fact that it already throws in 2 browsers should be enough.

[annevk]

It already seems to throw somewhat unreliably due to the exceptions from navigate, but if we think it's safer to return null instead I'd be happy to align on that too. Currently neither behavior is defined (and instead something else is required).

[jeisinger]

btw, looking at the blink sources, if you specify a target frame, then we'll return that frame instead of null for an invalid url. Is that expected?

[zcorpan]

I tested that in the link given in the OP. I get null in Chromium and WebKit for the iframe case. Is there another case where it does not return null?

[jeisinger]

sorry, I confused invalid with empty URL.

For completeness, _parent and _top are treated differently by blink, so WPT should cover those cases as well

[jeisinger]

Filed https://bugs.chromium.org/p/chromium/issues/detail?id=708135 to track this

[jeisinger]

use counter is pretty much 0%: https://www.chromestatus.com/metrics/feature/timeline/popularity/1909

we can still wait until it hits stable channel, but I think it's ok to just move forward here

[annevk]

I filed https://bugs.webkit.org/show_bug.cgi?id=171656 against WebKit.

[domenic]

Both Edge and Gecko fire the wrong exception type, so bugs on them would be good too.

[annevk]

• https://bugzilla.mozilla.org/show_bug.cgi?id=1362075
• https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/11898114/