Allow for relative paths when loading RTL plugin

[ryanhamley]

Launch Checklist

• briefly describe the changes in this PR
  • Allows for relative paths when loading RTL plugin
• write tests for all new functionality
• manually test the debug page

Closes #6719

[kkaefer]

Looks like we're doing something similar in

mapbox-gl-js/src/source/geojson_source.js

Lines 288 to 292 in 6255e9e

	function resolveURL(url) {
	const a = window.document.createElement('a');
	a.href = url;
	return a.href;
	}

Can we move that code to a more central place to avoid duplicating it?

[ryanhamley]

Ah thanks for pointing that out. Yeah, we should be able to centralize this somewhere.

[kkaefer]

Do we need to special case absolute URLs? It looks like we could also just happily pipe absolute/protocol-relative URLs through the a/href resolver. If we need this, it'd be a good idea to add the absolute URL special case to the GeoJSON resolver (see below) as well)

didn't mean to deny

[ryanhamley]

I'm not sure how to test this other than manually. There's no straightforward way to test this in the unit tests since Node doesn't have an window.location.origin and I don't think you can set it since origin is readonly. The render tests hardcode the plugin through an import statement and changing this to load the plugin via the setRTLTextPlugin method seems to break all the render tests. Any ideas? Am I overthinking this? It definitely works since it was tested on the debug page csp.html. @kkaefer

[jfirebaugh]

@ChrisLoer can correct me if I'm wrong, but I believe we've been relying on manual testing for the RTL plugin.

[ChrisLoer]

I believe we've been relying on manual testing for the RTL plugin.

Mainly, yes, and I think testing manually with csp.html is probably the most important/effective test for this pathway. The style unit test has a "register plugin listener" test case that could easily be modified to test the handling of a relative path -- but as @ryanhamley pointed out that gets tripped up on not having a window.location.origin. I think you could probably use Sinon's stubbing to handle that, but you'd be testing a pretty narrow/contrived case.

Thanks for fixing this, Ryan!