Currently the block is always limited to 1h. (Which by default is btw not respected for anonymous page caching).

There is no need for that if no backend URL is configured.

CommentFileSizeAuthor
#9 shariff-backend-url-cache-2975992-9.patch662 bytescriz
#2 shariff-backend-url-cache-2975992-2.patch1020 bytesberdir

Comments

Berdir created an issue. See original summary.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Needs review
StatusFileSize
new shariff-backend-url-cache-2975992-2.patch1020 bytes
criz’s picture

criz
he/him
Location Vienna/Austria
commented
Status: Needs review » Reviewed & tested by the community

Thanks, makes sense and looks good!

Which by default is btw not respected for anonymous page caching

This is not addressed by the patch, right? Any idea how this can be solved?

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

There's no easy way to solve that, in the end it's a core issue #2352009: Bubbling of elements' max-age to the page's headers and the page cache.

The reason we actually noticed this is that we have a custom response event subscriber that converts a non-zero (because too many things still set max-age 0 like the language switcher) max-age to the Expires header, which is the only thing that the internal page cache module respects.

That said, isn't the counter a separate JS request anyway and therefore completely separated from any kind of caching of the shariff html itself? In that case, this could actually just be removed completely..

criz’s picture

criz
he/him
Location Vienna/Austria
commented
Status: Reviewed & tested by the community » Needs review

Okay, thanks for the info!

I guess it is a separate JS request. But as I am not using the shariff backend so far I have to set it up and test to be sure.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Neither am I, but I'm pretty sure that is the case because nothing is included in the markup except the backend URL. which means it is always up to date and the cache limitation could be removed completely?

criz’s picture

criz
he/him
Location Vienna/Austria
commented
Title: Only limit block cache max age if a backend URL is configured » Reconsider using block cache max age
Status: Needs review » Needs work

Yes, seems so. :)

1an’s picture

1an commented
Assigned: Unassigned » 1an
criz’s picture

criz
he/him
Location Vienna/Austria
commented
Assigned: 1an » Unassigned
Status: Needs work » Needs review
StatusFileSize
new shariff-backend-url-cache-2975992-9.patch662 bytes
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

makes sense :)

  • criz committed 271d251 on 8.x-1.x 
    Issue #2975992 by Berdir, criz: Reconsider using block cache max age
criz’s picture

criz
he/him
Location Vienna/Austria
commented
Status: Reviewed & tested by the community » Fixed

Thanks!

Status: Fixed » Closed (fixed)

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