Time-based custom caching is broken since configuration expects an integer.

-1 seems a better choice than 7 to mean "custom".

However, isn't this really only a UI issue? Why do we need both results_lifespan and results_lifespan_custom? We could store only one value, and then display the custom setting if the value isn't in the list of dropdown options, we don't need to store both.

This seems related to, or perhaps a duplicate of, #3024941: The error in output select options in page "Caching options" after saving view

Why can't results_lifespan always store 0 for never cache, or the exact number of seconds? Then the UI can show the dropdown and hide or show the custom field depending on if the stored value is in the dropdown or not? This would simplify the logic of using the value at least.

There is nothing at present stopping you from saving a value that's not in the dropdown for results_lifespan, and it will work (except in the UI). There is also no difference between results_lifespan = 600 and results_lifespan = custom, results_lifespan_custom = 600.

LGTM, just one small suggestion:

+++ b/core/modules/views/src/Plugin/views/cache/Time.php
@@ -96,7 +96,8 @@ class Time extends CachePluginBase {
+    $options = [0 => $this->t('Never cache')] + $options + [-1 => $this->t('Custom')];

Could we make this -1 value a class constant for better DX? e.g. static::LIFESPAN_CUSTOM

This issue came up as a triage issue in #bugsmash

Nice work on this. #12 does look like the less invasive approach, but I think I like the proposed #6 more, getting rid of any setting sounds good to me, we have too many in Views anyway. It also feels like it would indeed simplify the logic a little.

Anyway, this needs test coverage and if we go for #6 it would need an upgrade path to get rid of the old settings.

Closed https://www.drupal.org/project/drupal/issues/3024941 as duplicate of this
Moving over credit for

asya_asina = initial patch
Aanal.addweb = first testing
Amber Himes Matz = triaging and discovering the duplicate

Moving to NW for the tests

Verified the issue exists in 9.5
Verified the patch in #12 still applies and fixes the issue.

Not sure if some kind of notice will be needed since site admins will have to go in and fix their views if currently using that setting.

PHPCS failed:

FILE: /var/www/html/core/modules/views/src/Plugin/views/cache/Time.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 155 | WARNING | [x] A comma should follow the last multiline array
     |         |     item. Found: $output_default

Also will still need tests.

So the current question is the approach in #25 better than #12.

Adding Needs subsystem maintainer review to get their review on this as I don't have much an opinion on either approach.

So after testing it seems #12 is the more complete solution.

#25 when I exported my view it didn't show that I had a custom setting. It exported the seconds but if those seconds were say 3600 the view would load that as non custom.

Adding a test

Here's the patch I meant to upload. Just uppercase C

I have run into this again on a project, #31 works for me and is the simplest way to fix this; we can improve this later in followup issues.

Note that usually this sort of change would require an upgrade path but currently this feature is broken and views are not saved correctly when the custom option is used, so I don't think an upgrade is viable or useful here.

I think given this is quite broken we should consider how this works. I think saving both results_lifespan_custom and results_lifespan (and the duplicate for output_lifespan*) is really odd. We should be saving on lifespan.

The logic could be something like this. If the lifespan is not in the preset list then set it to the custom lifespan option and set the default value of the test field. On save if the lifespan is set to custom then replace it with the integer from the custom lifespan field. That way the final view has less fields and makes more sense.

Sorry looked at the patch directly... didn't read #35. Sorry @longwave. As per #36 I think that considering this is totally broken we should take the opportunity to fix it in the best way possible rather than do it in steps because it's not as if it is partially working.

Similar bug with more details and different explanations at #3371265: Views time-based caching stores and custom duration as zero, marking as duplicate since this one already has a patch in the works.

Patch isn't applying to 10.1.8. Needing 24 hours.

$options = [60, 300, 1800, 3600, 21600, 86400, 518400];

Hi,
Rerolled patch #31 for 11.x branch, please review it.
Thanks!

Created MR for the changes in patch #44, please review.

Pipeline has issues.

Would recommend checking that everything is green before putting into review.

Fixed pipeline issues and updated MR, please review it.
Thanks!

This still uses two separate config keys, which isn't necessary - to make the config simpler we should implement @alexpott's suggestion from #36.

Cross-referencing #3609866: Views Time cache "Never cache" label is misleading and caches permanently which could impact this issue.

Implemented #36 with help from Claude Code. The _custom fields are gone, there are now just results_lifespan and output_lifespan keys that store the raw value, the UI handles the selection of preset vs custom values.

The update hook removes _custom fields and preserves existing behaviour: only if the raw value is the string "custom" then the custom value is actually used.

Sorry can we clean up the summary some? Solution says option7?

Updated IS.

I have reviewed the MR and tested manually. Didn't find anything major IMO, so I think we should be close to ready here.
Setting as NW just to update the CR URL in the MR, then to me this is ready for RTBC