Problem/Motivation

The image URLs can be different based on the Accept header sent by the browser, but since the cache context for the Accept header is not being added, whatever is rendered first will be cached for all browsers. This leads to browsers not supporting WebP showing WebP images, or browsers supporting WebP not showing the WebP version.

Proposed resolution

Add the headers:accept cache context.

CommentFileSizeAuthor
#14 3212705-14.patch2.96 KBduwid
#13 3212705-13.patch2.96 KBseanb

Comments

seanB created an issue. See original summary.

  • 93be963 committed on 1.0.x 
    Issue #3212705: Add cache context for accept header
seanb’s picture

seanb
Primary language Dutch
Location Netherlands
commented
Status: Active » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

kubrick’s picture

kubrick commented

I'm still having this issue on 1.1.

Whichever version (jpg/webp) gets hit first gets cached and served to every user.

kubrick’s picture

kubrick commented

Still can't get this to work. If a webp capable browser hits the page first it caches the page with webp url's for all visitors. (and vice versa)

Not that much of an issue these days as there's not a lot of users using on browsers that don't support webp.

kubrick’s picture

kubrick commented

I've looked into this again, and it seems if you have page caching enabled Drupal will always serve the same page to all anonymous users, regardless of headers.

https://www.drupal.org/docs/8/api/cache-api/cache-contexts#internal

Maybe it would be an option to always render WebP by default (if module available) and have a JavaScript based fallback load the JPG version on older browsers.

seanb’s picture

seanb
Primary language Dutch
Location Netherlands
commented
Status: Closed (fixed) » Active

Since most browsers support WebP, I would prefer not to have a lot of code to deal with this issue. However, since Internal Page Cache could have significant benefits for smaller sites, I think it would make sense to have something to deal with this.

We could add a checkbox to the settings page when a WebP module AND Internal Page Cache are enabled. When the checkbox is enabled, we skip the HTTP_ACCEPT header check and always serve WebP. Would this help your use case?

kubrick’s picture

kubrick commented

Sure, that would work. :)

seanb’s picture

seanb
Primary language Dutch
Location Netherlands
commented

I'm slowly thinking that we should remove the check for the HTTP_ACCEPT header now most browsers support WebP. Any thoughts?

kubrick’s picture

kubrick commented

Yes, I agree. I see no harm in just serving WebP to all users if one of the WebP modules is enabled.

seanb’s picture

seanb
Primary language Dutch
Location Netherlands
commented

Could you create a PR for this?

seanb’s picture

seanb
Primary language Dutch
Location Netherlands
commented
Status: Active » Needs review
StatusFileSize
new 3212705-13.patch2.96 KB

Created a patch that should fix this.

duwid’s picture

duwid commented
StatusFileSize
new 3212705-14.patch2.96 KB

Thank you for the patch. I rerolled it to work with version 1.3.1

  • seanB authored ced7e669 on 1.3.x 
    Issue #3212705 by seanB, Duwid, kubrick: Add cache context for accept...
seanb’s picture

seanb
Primary language Dutch
Location Netherlands
commented
Status: Needs review » Fixed

Thanks, went ahead and committed this. All major browsers support WebP by now. No need to serve WebP conditionally anymore.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.