Google is abandoning FLoC - so remove the header

Or maybe replace it with the header to block the Topics API, because in the end it’s more of the same.

#3284706: Error in Edge browser - Error with Permission-policy header: origin trial controlled feature not enabled 'interest cohort'

LGTM

Just to note that the Topics API doc at https://github.com/patcg-individual-drafts/topics states:

A site can forbid topic calculation on a page via the following permission policy header:
Permissions-Policy: browsing-topics=()
Note: The old Permissions-Policy: interest-cohort=() from FLoC will also forbid topic calculation.

Unsure if we want to switch to this instead, or whether Topics API is less controversial so it should be up to individual sites via e.g. https://www.drupal.org/project/permissionspolicy

We need to deprecate the setting then and tell users to clear it up. This will make @catch happy because we left the settings deprecation stuff in D10.

I wasn't able to apply this patch to 9.4.8 release using composer-patch. Still in my headers though.

$ git apply -p1 --directory='web' 3260401-5.patch
error: patch failed: web/core/assets/scaffold/files/default.settings.php:593
error: web/core/assets/scaffold/files/default.settings.php: patch does not apply
error: patch failed: web/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php:131
error: web/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php: patch does not apply
error: web/core/tests/Drupal/KernelTests/Core/Http/BlockInterestCohortTest.php: No such file or directory
error: patch failed: web/sites/default/default.settings.php:593
error: web/sites/default/default.settings.php: patch does not apply

Misunderstood bot change.

1. Added a change record at https://www.drupal.org/node/3320787
2. Added a deprecation \Drupal\Core\Site\Settings::$deprecatedSettings instructing developers to remove the block_interest_cohort Drupal setting.
3. Added a deprecation assertion to \Drupal\Tests\Core\Site\SettingsTest::providerTestRealDeprecatedSettings

+++ b/core/lib/Drupal/Core/Site/Settings.php
@@ -37,7 +37,12 @@ final class Settings {
+      'message' => 'The "block_interest_cohort" setting is deprecated in drupal:9.5.0 and will be removed in drupal:9.5.0. This setting should be removed from the settings file. See https://www.drupal.org/node/3320787.',

Can we word this a bit differently? It's confusing to repeat the same version, although I know that is our standard format; we don't generally expect to deprecate and remove at the same time - though it makes sense here.

#16 Reworded to: The "block_interest_cohort" setting is deprecated in drupal:9.5.0. This setting should be removed from the settings file, since its usage has been removed. See https://www.drupal.org/node/3320787.

This looks great to me for 10.x - but it doesn't apply to 9.5.x. Can you also roll a version for that, and we can hopefully get this in to the 9.5.0 release?

trying to fix in 9.5.x address #19

fixing CCF Needs review

Thanks, this all looks good now.

#23 is for 9.5.x.
#18 is for 10.0.x/10.1.x.

I went to try and commit this, and run into what I think is a completely unrelated phpstan issue:

----------------------------------------------------------------------------------------------------

Running PHPStan on changed files.
 ------ ------------------------------------------- 
  Line   sites/default/default.settings.php         
 ------ ------------------------------------------- 
  647    Variable $app_root might not be defined.   
  647    Variable $site_path might not be defined.  
 ------ ------------------------------------------- 


                                                                                
 [ERROR] Found 2 errors                                                         
 

Could probably commit it without running phpstan, but we will need an issue to sort that out either way. Might need a phpstan-ignore maybe.

Hmm, when PHPStan is running on all files, per usual on Drupal CI, nothing is detected:

00:03:03.782 core/scripts/dev/commit-code-check.sh --drupalci

00:03:24.008 
1/4 ./assets/scaffold/files/default.settings.php 1049.17ms
00:03:25.346 
2/4 ./lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php 118.53ms
00:03:25.465 
3/4 ./lib/Drupal/Core/Site/Settings.php 87.24ms
00:03:25.554 
4/4 ./tests/Drupal/Tests/Core/Site/SettingsTest.php 89.67ms
00:03:25.644 CSpell: Files checked: 4, Issues found: 0 in 0 files
00:03:25.668 
00:03:25.668 CSpell: passed
00:03:25.668 
00:03:25.669 ----------------------------------------------------------------------------------------------------
00:03:25.669 
00:03:25.669 Running PHPStan on *all* files.

00:06:22.453 
00:06:22.459  [OK] No errors                                                                 
00:06:22.459 
00:06:22.487 
00:06:22.487 PHPStan: passed

Since DrupalCI can't replicate the problem, I can random add phpstan-ignore-next-line, but that seems a bit weird.

Maybe a core committer has to fix this themself?
And why are the differences between checking all and only changed files with PHPStan?

So these are in the baseline for the scaffold source version:

                -
                        message: "#^Variable \\$app_root might not be defined\\.$#"
                        count: 1
                        path: assets/scaffold/files/default.settings.php

                -
                        message: "#^Variable \\$site_path might not be defined\\.$#"
                        count: 1
                        path: assets/scaffold/files/default.settings.php

but not for the copy in sites/default/default.settings.php - but I can't immediately see why this is.

We already ignore settings.php, maybe we need to extend this to default.settings.php, given that it's an include and testing it standalone doesn't make sense?

    # Skip settings.
    - ../*/settings*.php

Oh, I see:

  paths:
    - .
    - ../composer

phpstan.neon.dist tells PHPStan to only analyse the core and composer top level directories by default, but not sites - however the precommit script doesn't take this into account, so if you explicitly specify a file outside of the listed paths, it will analyse it anyway; locally I get the same errors if I do this:

$ ../vendor/bin/phpstan analyze ../sites/default/default.settings.php
Note: Using configuration file /var/www/html/drupal/core/phpstan.neon.dist.
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ------------------------------------------- 
  Line   sites/default/default.settings.php         
 ------ ------------------------------------------- 
  662    Variable $app_root might not be defined.   
  662    Variable $site_path might not be defined.  
 ------ ------------------------------------------- 


                                                                                                                        
 [ERROR] Found 2 errors                                                                

Will open a followup to extend excludePaths to include default.settings.php.

Ok, so do we add the errors to the baseline, or ignore the default.settings.php?

I like the latter more.
Added a patch to see if that would work.

Re #32 I think that is technically out of scope here so opened #3322182: Ignore sites directory in PHPStan

Since the phpstan issue is unrelated, I've gone ahead and committed this to 10.1.x/10.0.x/9.5.x with --no-verify to skip the phpstan check on commit.

Published the change record at https://www.drupal.org/node/3320787