Harden/modernize JS in WP_Customize_Manager::remove_frameless_preview_messenger_channel()

[nikkifurls]

Hardens/modernizes the JavaScript in WP_Customize_Manager::remove_frameless_preview_messenger_channel(), leveraging URL instead of document.createElement('a').

Trac ticket: https://core.trac.wordpress.org/ticket/59480

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[dmsnell]

Looks sound to me. I didn't test this, but by inspection it looks fine. Support across all major browsers seems to predate 2017.

[westonruter]

In regards to anyone who may have concerns about the use of const here (which breaks in IE11), I'll note that frontend JS code from blocks now makes use of const, including the JS for the File, Image, Navigation, and Query blocks. A reminder that Internet Explorer is no longer supported by WordPress.

[westonruter]

I'm getting a static analysis warning here since url.search is supposed to be a string and queryParams is a URLSearchParams object:

I think actually there is no need for this line at all, since url.searchParams is updated above and mutating it appears to also update the underlying URL object.

In the spec for the URL Standard, it says that the searchParams property is [SameObject] as the URL: https://url.spec.whatwg.org/#url-class

See definition of SameObject doesn't really seem to indicate that mutation is propagated up to the "parent" object. Nevertheless, look at this example in the URL spec:

const url = new URL('https://example.com/?a=b ~');
console.log(url.href);   // "https://example.com/?a=b%20~"
url.searchParams.sort();
console.log(url.href);   // "https://example.com/?a=b+%7E"

So you can see that it is intended for mutation to propagate.

See another standalone example: https://codepen.io/westonruter/pen/YzdjzqG?editors=0010

So yeah, unless you see otherwise I think this line should be removed:

Suggested change

url.search = queryParams;

[dmsnell]

oh wow. I didn't test this but I assumed it cloned the search params since it mentions that property is read-only.

yikes. thanks for checking on this. sounds like we should add a comment explaining that the method calls on queryParams modify the URL, or possibly we should eliminate queryParams entirely and operate on url.searchParams.has( … ) and url.searchParams.delete( … )?

[westonruter]

Yes, good idea. Might as well eliminate the queryParams variable.

[nikkifurls]

Thank you both very much. I just made that change 👍

[westonruter]

Committed in r56749. Congratulations on your first core contribution!