Closes #3423 Add preconnect tag for CDN URLs

[iCaspar]

Closes #3423

• Add add_precnnect_cdn() method to wp_resource_hints filter.
• Add integration tests for new method.

[iCaspar]

[WIP] @see note on #3423 -- need to re-think how to add more than 1 instance of the preconnect tag for a domain -- wp_resource_hint() allows only 1.

[iCaspar]

@Tabrisrp @GeekPress I've gone with basically what Jonathan put here as option 2, namely the function we will run on the wp_resource_hints filter will return the CDN domains with and without the crossorigin="anonymous", such that the non-crossorigin will be printed as things stand right now, and both will be printed if ever core adopts Jo's patch.

[Mai-Saad]

@iCaspar while minify CSS enabled, is this fine?
1- When CDN is domain.com i.e example.org then the href for preconnect will be over HTTP not HTTPS i.e
<link href='http://example.org' crossorigin='anonymous' rel='preconnect' /> while the css file itself is HTTPS
2- When CDN is https://example.org , there is no crossorigin for preconnect
<link href='https://example.org' rel='preconnect' />

[iCaspar]

@Tabrisrp On @Mai-Saad question 1 above, can you confirm whether that is correct, or should we always force a CDN url link to use https? As is, it uses whatever the customer has entered into the list of CDNs in the dashboard.

On 2, that is unfortunately the expected, though not ideal, behavior for now. (see above)

[Tabrisrp]

@iCaspar What do you think of this approach:

• Use the URL with the protocol if it's defined in the CDN value (http(s)://cdn.example.org)
• Use a protocol-relative URL if the protocol is not defined in the value (//cdn.example.org)

[iCaspar]

@Tabrisrp Implemented your suggestion above. I think the new testcases should cover things, but maybe there are others.

[iCaspar]

@Mai-Saad @wp-media/qa - Revised now to use either the specified schema or simply // if no schema is provided by the user.

[Mai-Saad]

@iCaspar @Tabrisrp
1- Shall we have any kind of validation on CDN that includes at least a single dot and not empty? ( currently, if CDN is test/path then we will use //test as href)
2- Using CDN name as /test/path will result in applying this to all files i.e file href will be: https:///test/path/wp-content/cache/busting/1/gtm-5baa953518f36f664ef5d618f4d839a2.js also no dns-prefetch nor preconnect .. I think it shouldn't be applied to files, should it?
3- Currently if the CDN name is http://test.com then
dns-prefetch is <link rel='dns-prefetch' href='//test.me' />
while preconnect is : <link href='http://test.me' rel='preconnect' /> and same for files... so is this fine or shall dns-prefetch be as same as preconnect here?
4- Currently, changing the CDN name will delete minified/combined files and these files won't come back till clear cache or deactivate the plugin, in this case, a new minified file shall be auto-created, right? (same behavior on the trunk, I guess we can open a separate issue for this)

[iCaspar]

On 1 and 2: The preconnect is for a domain, not a file, as we are only concerned about connecting to the server, not accessing anything specific. So, the expected behavior should be to ignore any CDN entry that a is single file or directory, and it should remove any path after the domain host part of the URL.
On 3: The dns-prefetch links are generated from WordPress, not by us, a different method related to CDN, and will always print the //test.me version. I think it will be fine to use the customer's scheme, http(s), if they have specified it for any reason, as prefetch and preconnect are different things. In prefetch only the DNS is resolved, but in preconnect, an actual connection with the server is established, so having the specific schema to make connect could be helpful if the customer provides it.
On 4: Since that is already the behavior, we should handle it separately as another issue, yes. If you would go ahead and open that, would be awesome! Thanks.

[Tabrisrp]

On 4) When changing the CDN cname value, everything is deleted: cache files & minify/combine files, so there is nothing to do. The new cache & minify files will be generated on the next visit (or preload).

[Mai-Saad]

@Tabrisrp on 4:
for minify css -> cached files in min are autogenerated currently without a new visit after changing CDN name. While for minify js -> cached files in min aren't autogenerated even with a hard refresh of the web page.
Scenario1 => Reproduce steps:
1- nothing enabled in file optimization + CDN enabled
2- clear cache
3- enable minify js
4- visit the home page -> check min folder in cache "js files added in 1 folder"
5- change CDN and save -> here the js files in min/1 are deleted
6- hard refresh the home page
7- Check cache/min/1 -> js files not added

Scenario 2 => steps:
1- minify js enabled + CDN not enabled
2- visit the home page and check min/1 folder "js files are there"
3- enable CDN and check min/1 folder "js files removed"
4- visit the home page and check min/1 folder " js files still not added"

[Tabrisrp]

Probably this bug: #2973

[Mai-Saad]

Probably this bug: #2973

@Tabrisrp I think it`s related but not sure if they are the same thing as even if minify js was enabled and working fine before enable/edit CDN, js files won't be regenerated till clear cache and visit the URL again

[iCaspar]

In further conversation with @Mai-Saad, it turns out we were not eliminating a CDN entry like test/tests (though we were for /test/tests). @Tabrisrp, I've added a regex to skip those here. We have the same issue in the add_dns_prefetch_cdn() and probably get_cdn_hosts(). The latest change solves for the preconnect, since that's what's new here. Not catching it in prefetch and possibly other places can be a separate issue, not to hold this up getting into 3.8.6.

[Mai-Saad]

CDN PR all handled now except for
1- ignore test , .com
2- add preconnect for //test.com
Note: we don't add crossorigin anymore also it was there with CDN name example.org

[iCaspar]

@wp-media/qa Per conversation with @piotrbak and @Tabrisrp we're going to go with this as is and start a new issue to validate CDN values when the field gets saved in order to prevent invalid domains/hosts from being in the picture on this and in several other places.