WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 23 months ago

#38121 closed enhancement (fixed)

Crossorigin and other attributes for wp_resource_hints

Reported by: superpoincare Owned by: swissspidy
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.6
Component: Script Loader Keywords: has-patch has-unit-tests needs-dev-note commit
Focuses: performance Cc:

Description

At present, wp_resource_hints doesn't allow various attributes that can be added to the link element. One example is preconnect.

This is required for preconnect to Google fonts to work.

See this example: https://www.webpagetest.org/video/compare.php?tests=160921_P0_2GSV,160921_QP_2GSW

Related ticket: https://core.trac.wordpress.org/ticket/37171

Hence I am opening this ticket: for enhancement to wp_resource_hints

Attachments (5)

38121.diff (5.6 KB) - added by swissspidy 3 years ago.
38121.2.diff (5.6 KB) - added by peterwilsoncc 3 years ago.
38121.3.diff (5.6 KB) - added by peterwilsoncc 3 years ago.
38121.4.diff (5.7 KB) - added by swissspidy 3 years ago.
38121.5.diff (4.3 KB) - added by swissspidy 3 years ago.

Download all attachments as: .zip

Change History (23)

#1 @peterwilsoncc
3 years ago

  • Version changed from 4.6.1 to 4.6

Any thoughts on this @swissspidy & @voldemortensen?

I'm inclined to be opinionated and include the crossorigin attribute where needed given the aim is to reduce lag cross origin.

#2 follow-up: @swissspidy
3 years ago

Attributes have come up before, but we didn't really have time to consider all of them, especially since the specification is still in draft status.

There's the hint probability (pr) attribute (https://www.w3.org/TR/resource-hints/#attributes) as well as the as attribute. crossorigin can also have a value, e.g. crossorigin="use-credentials", see https://html.spec.whatwg.org/multipage/infrastructure.html#cors-settings-attributes.

Also, we might want to support preload as well.

Of course, this all would need to be done in a backward compatible way.

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

Replying to swissspidy:

Of course, this all would need to be done in a backward compatible way.

A couple of possibilities, each attribute can be a string (URL only) or one of the following:

'example.com' => array( 
  'cross-origin' => 'user-credentials', 
  'other' => 'atts'
),

or

array(
  'href' => 'example.com',  //alias with url
  'cross-origin' => 'user-credentials',
  'other' => 'atts'
),

an is_array check is used to determine if the additional attributes exist.

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


3 years ago

#5 @swissspidy
3 years ago

  • Milestone changed from Awaiting Review to 4.7
  • Owner set to swissspidy
  • Status changed from new to assigned

@swissspidy
3 years ago

#6 @swissspidy
3 years ago

  • Keywords has-patch has-unit-tests added

#7 @swissspidy
3 years ago

@peterwilsoncc @voldemortensen What do you folks think of the latest patch + tests?

#8 @peterwilsoncc
3 years ago

The once change I'd make to your patch is to replace a number if else ifs with elseif, see 38121.2.diff.

Seeing it code, I'm not sure my first suggestion above is a good idea afterall. It seems a bit error prone. I'll work on something seperate.

#9 @peterwilsoncc
3 years ago

38121.3.diff uses the second format from comment:3. No url alias for href as it's not done elsewhere.

#10 @swissspidy
3 years ago

  • Keywords needs-dev-note added

@swissspidy
3 years ago

#11 @swissspidy
3 years ago

38121.4.diff fixes a couple of bugs from the previous patches. Also makes sure that invalid entries with no href attribute are skipped.

#12 @peterwilsoncc
3 years ago

  • Keywords commit added

LGTM, minor styling issue in attribute loop with ... && ! is_numeric( $attr )) but apart from that looks ready for commit.

#13 @ocean90
3 years ago

Shouldn't preload be handled in a separate ticket? See also https://github.com/w3c/preload/issues/76 for current browser support.

#14 @swissspidy
3 years ago

@ocean90 I'd usually agree, but these two specs are kinda tied together. See the as attribute as an example:

The as attribute is an optional attribute that must conform to requirements defined in PRELOAD.

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


3 years ago

@swissspidy
3 years ago

#16 @swissspidy
3 years ago

Actually, ignore my previous comment. preload is really not the original goal of this ticket. It deserves its own ticket + discussion.

See 38121.5.diff.

Last edited 23 months ago by swissspidy (previous) (diff)

#17 @swissspidy
3 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 38826:

Resource Hints: Allow passing custom attributes to resource hints.

[37920] introduced resource hints that allow browsers to prefetch specific pages or render them in the background. With this change, the as, crossorigin, pr, and type attributes can be passed in addition to the URLs/hosts.

Props peterwilsoncc, swissspidy.
Fixes #38121.

This ticket was mentioned in Slack in #core-themes by davidakennedy. View the logs.


3 years ago

Note: See TracTickets for help on using tickets.