Quicklink detects session when there is none in Drupal 8.6.14 and above

Yea, the patch doesn't make sense to me and seems like it should be working as-is. Is this a vanilla Drupal site you're testing or something that might be using private storage or other session invoking code?

+++ b/quicklink.module
@@ -153,7 +153,7 @@ function quicklink_preprocess_html(&$variables, $hook) {
     $session = \Drupal::getContainer()->get('session_manager');
-    if (!empty($session) && $session->isStarted()) {
+    if (!empty($session) && $session->start()) {

The session manager should never be empty.

👀looks like you could use some help writing some tests to catch this.

Here are three patches. The last one using session_configuration should work, and is why page_cache did not break in Drupal core -- this is how it detects a session.

Here's a patch which has a test that shows HEAD has a failure.

This uses a setup script. And fixes another missing config item in the schema.

Here's a patch with the fix. However, please don't commit this just yet. Tomorrow I'd like to provide a proper patch for the schema.yml. Or at least fix the labels in this one.

Here is a reroll. I made one against 8.x-1.x and the 8.x-1.x-dev (which shouldn't exist?)

FWIW you probably need to merge 8.x-1.x-dev into 8.x-1.x and then delete the 8.x-1.x-dev branch, then patch two from #20 will apply.

Yep, you want to use SessionConfigurationInterface::hasSession(), this is also what BigPipe uses to only activate itself for requests with sessions (see https://git.drupalcode.org/project/drupal/blob/c779a82a28c2e9f140f6a11db...).

So the committed solution should work fine! 👍

While looking at the code, I did spot two problems, both relating to cacheability:

1. $variables['#cache']['contexts'][] = 'session.exists'; should be inside the "if no_load_when_session" branch, to vary correctly — BigPipe does this too (see https://git.drupalcode.org/project/drupal/blob/c779a82a28c2e9f140f6a11db...) — this is still directly related to the originally reported problem
2. $variables['#cache']['tags'] = $config->getCacheTags(); should be added immediately after loading $config to ensure that configuration changes are reflected immediately, without manual clearing of caches :) — this one I just noticed by looking at everything above.