Closed (duplicate)
Project:
Cache Control Override
Version:
8.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
20 Apr 2018 at 12:23 UTC
Updated:
21 Mar 2025 at 10:47 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
grimreaperHere is the patch.
Thanks for the review.
Comment #3
grimreaperSame patch but made against the 8.x-1.0-alpha1 tag.
Comment #4
grimreaperSorry, I had not tested the dependency injection. Here are two new patches that do it properly.
Comment #5
mark_fullmerThe previous patches weren't in the right format to be apply-able; I've recreated the same changes in the attached patch. I've also verified the fix, though I'll forego RTBC'ing this, since I'm adding a patch now.
For future reference, a quick manual verification of this (in the absence of automated tests) is below:
A content block placed on a page will yield the following:

.... while a page without that block (assuming a global TTL set to "60" will show:

Importantly, I think this "Expires" behavior was not the original intent of the module (rather, it was intended for reverse proxies & edge caches; however, I think there are valid use cases for letting the "Expires" value bubble up to the Internal Page Cache's cache_page bin, as this change accomplishes (e.g., a widget that is responsible for retrieving external content every X minutes).
Comment #6
leon kessler commentedI don't see why this patch should not be incorporated into the project. Yes most people will be using this module with an external cache/CDN, and probably have the internal page cache module uninstalled.
However, for those that need to use the page_cache module, it's either this patch, or patching Drupal core with https://www.drupal.org/project/drupal/issues/2352009#comment-14064154 (which is the latest patch on that issue at this current time).
I don't believe there is any harm in setting the correct Expires header, but I suppose there could be some unexpected consequences for someones setup (although you could argue if this does cause an issue for someone, it's a symptom of a bug in their infrastructure, rather than with the Drupal instance itself).
Comment #7
feng-shui commentedRe-roll against 2.x.
Comment #8
prudloff commentedWe should probably decide which patch to keep between #2962699: Expires header not set and #2916705: Page cache isn't invalidated.
Comment #9
prudloff commentedClosing as a duplicate of #2916705: Page cache isn't invalidated, the two patches basically do the same thing and the other one has a more recent patch.
Comment #10
prudloff commented