WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#37652 closed defect (bug) (fixed)

Protocol-relative preconnects are broken

Reported by: neoxx Owned by: ocean90
Milestone: 4.6 Priority: normal
Severity: normal Version: 4.6
Component: Script Loader Keywords: has-patch has-unit-tests commit
Focuses: performance Cc:

Description

Following https://w3c.github.io/resource-hints/ preconnect supports protocol-relative URLs.

The current implementation removes the double-slashes and you'll end up with

<link rel='preconnect' href='example.org'>

instead of

<link rel='preconnect' href='//example.org'>.

A real-world test in Chrome confirms that such a preconnect does not work.

Attachments (4)

37652.diff (2.1 KB) - added by swissspidy 3 years ago.
37652.2.diff (2.8 KB) - added by peterwilsoncc 3 years ago.
37652.3.diff (3.2 KB) - added by peterwilsoncc 3 years ago.
37652.4.diff (3.4 KB) - added by ocean90 3 years ago.

Download all attachments as: .zip

Change History (20)

#1 @swissspidy
3 years ago

The current implementation removes the double-slashes and you'll end up with

How did you add the URL? Some example code would be helpful.

#2 @neoxx
3 years ago

Sure thing, here you go:

function wp_resource_hints($hints, $relation_type) {
        if ($relation_type=='preconnect') {
                $hints[]='//example.org';
        }

        return $hints;
}

Though, I currently use the following work-around:

$protocol=(is_ssl()) ? 'https' : 'http';

$hints[]=$protocol.'://example.org';

@swissspidy
3 years ago

#3 follow-up: @swissspidy
3 years ago

  • Keywords has-patch has-unit-tests added

Thanks a bunch for the snippet. 37652.diff should fix this.

@voldemortensen What do you think about this?

#4 in reply to: ↑ 3 @peterwilsoncc
3 years ago

  • Milestone changed from Awaiting Review to 4.6

Replying to swissspidy:

Thanks a bunch for the snippet. 37652.diff should fix this.

The // prefix can go on the else, the code is nested so only runs on preconnect & dns-prefetch.

I'll upload what I had for the sake of completeness momentarily.

#5 @peterwilsoncc
3 years ago

  • Milestone changed from 4.6 to Awaiting Review

Reverting itchy trigger finger.

#6 @swissspidy
3 years ago

Thanks @peterwilsoncc. We should combine the tests of both patches though as they do slightly different things. For example, test_preconnect() should test various URLs and not just one.

#7 @ocean90
3 years ago

  • Milestone changed from Awaiting Review to 4.6

We're going to break code freeze with #36819 so we can fix this too.

This ticket was mentioned in Slack in #core by ocean90. View the logs.


3 years ago

#9 @peterwilsoncc
3 years ago

Combined version in 37652.3.diff, preconnect test comes from 37652.diff, other code and tests from 37652.2.diff.

#10 @ocean90
3 years ago

  • Keywords commit added

37652.3.diff looks good to me.

@ocean90
3 years ago

#11 @ocean90
3 years ago

In 37652.4.diff I've inverted the condition to merge both $url = '//' . $parsed['host']; lines into one.

This ticket was mentioned in Slack in #core by ocean90. View the logs.


3 years ago

#13 @azaozz
3 years ago

37652.4.diff looks good, +1 to commit.

#14 @ocean90
3 years ago

  • Owner set to ocean90
  • Resolution set to fixed
  • Status changed from new to closed

In 38255:

Script Loader: Fix protocol-relative URLs for the preconnect relation type.

wp_resource_hints() parses the URL for the preconnect and dns-prefetch relation types to ensure correct values for both. While protocol-relative URLs are supported for dns-prefetch, the double slash was lost for preconnect.

Props swissspidy, peterwilsoncc.
Props azaozz for review.
Fixes #37652.

#15 @ocean90
3 years ago

In 38256:

Script Loader: Fix protocol-relative URLs for the preconnect relation type.

wp_resource_hints() parses the URL for the preconnect and dns-prefetch relation types to ensure correct values for both. While protocol-relative URLs are supported for dns-prefetch, the double slash was lost for preconnect.

Merge of [38255] to the 4.6 branch.

Props swissspidy, peterwilsoncc.
Props azaozz for review.
See #37652.

This ticket was mentioned in Slack in #core by voldemortensen. View the logs.


3 years ago

Note: See TracTickets for help on using tickets.