Deprecate drupal_http_header_attributes()

Fits into prioritized https://www.drupal.org/contribute/core/beta-changes#prioritized
"dead code"

Okay!

removed!

sorry! wrong status

sorry! wrong status

It seems like this function is still in use

Yep

$ git grep drupal_http_header_attributes
core/includes/common.inc:372:function drupal_http_header_attributes(array $attributes = array()) {
core/lib/Drupal/Core/Render/HtmlResponseAttachmentsProcessor.php:423:        $attached['http_header'][] = ['Link', $href . drupal_http_header_attributes($attributes), TRUE];

Reroll #7

drupal_http_header_attributes() is still called by HtmlResponseAttachmentsProcessor::processHtmlHeadLink()

hi,
What should be tested?
it's a protected method.

Here is the patch in #27 with the fixes on #29 but the mock on 3th paragraph.

+++ b/core/includes/common.inc
@@ -361,26 +361,6 @@ function date_iso8601($date) {
-function drupal_http_header_attributes(array $attributes = array()) {
-  foreach ($attributes as $attribute => &$data) {
-    if (is_array($data)) {
-      $data = implode(' ', $data);
-    }
-    $data = $attribute . '="' . $data . '"';
-  }
-  return $attributes ? ' ' . implode('; ', $attributes) : '';
-}

We will need keep this function with @deprecated but it can just be a wrapper for a class method.

A wrapper is not possible because the method is protected
and i'm not sure if it make sense to change this.

reroll #30

Having a duplicate code in core will likely cause fragmentation if there is a bug fix and not remembered(we have one of these issues already in core with render escaping @see theme_render_and_autoescape()). If we can't make that method public for whatever reason, might as well make a BC public method that returns the result and marked as @internal or something.

+++ b/core/includes/common.inc
@@ -351,6 +351,8 @@ function date_iso8601($date) {
+ * @deprecated in Drupal 8.0.0, will be removed before Drupal 9.0.0.
...
 function drupal_http_header_attributes(array $attributes = array()) {

+++ b/core/lib/Drupal/Core/Render/HtmlResponseAttachmentsProcessor.php
@@ -417,13 +417,33 @@ protected function processHtmlHeadLink(array $html_head_link) {
   /**
+   * Formats an attribute string for an HTTP header.
+   *
+   * @param $attributes
+   *   An associative array of attributes such as 'rel'.
+   *
+   * @return
+   *   A separated string ready for insertion in a HTTP header. No escaping is
+   *   performed for HTML entities, so this string is not safe to be printed.
+   */
+  protected function formatHttpHeaderAttributes(array $attributes = array()) {

What about the following: a) Use 8.3.x here b) make the method public static but call it @internal and @deprecated, as in 9.x we can call to it internally

1. +++ b/core/lib/Drupal/Core/Render/HtmlResponseAttachmentsProcessor.php
   @@ -421,13 +421,37 @@ protected function processHtmlHeadLink(array $html_head_link) {
   +   *
   +   * @param $attributes
   +   *   An associative array of attributes such as 'rel'.
   +   *
   +   * @return
   +   *   A separated string ready for insertion in a HTTP header. No escaping is
   +   *   performed for HTML entities, so this string is not safe to be printed.
   +   *
   +   * @internal Used by processHtmlHeadLink. Should not be used
   +   *           in user code.
   +   * @deprecated in Drupal 8.3.x, will be removed before Drupal 9.0.
   

   Is there a reason we don't just copy the existing documentation?

2. +++ b/core/lib/Drupal/Core/Render/HtmlResponseAttachmentsProcessor.php
   @@ -421,13 +421,37 @@ protected function processHtmlHeadLink(array $html_head_link) {
   +   * @param $attributes
   ...
   +   * @return
   

   Should be @return string and @param array attributes

3. +++ b/core/lib/Drupal/Core/Render/HtmlResponseAttachmentsProcessor.php
   @@ -421,13 +421,37 @@ protected function processHtmlHeadLink(array $html_head_link) {
   +   * @deprecated in Drupal 8.3.x, will be removed before Drupal 9.0.
   

   Maybe explain why its protected here.

Rerolled, adapted to the Drupal deprecation policy and added CR.

I we want to keep the new method protected, we can add a deprecated public static method that just wraps the protected. But I think we can keep it public.

Fix a coding standards issue.

Looks ready, only question about docs & interface

+++ b/core/lib/Drupal/Core/Render/HtmlResponseAttachmentsProcessor.php
@@ -431,6 +431,26 @@ protected function processHtmlHeadLink(array $html_head_link) {
+   * Formats an attribute string for an HTTP header.
...
+  public static function formatHttpHeaderAttributes(array $attributes = []) {

Maybe move documentation to \Drupal\Core\Render\AttachmentsResponseProcessorInterface

Moved.

Oh, the patch :)

+++ b/core/lib/Drupal/Core/Render/AttachmentsResponseProcessorInterface.php
@@ -51,4 +51,16 @@
+  public static function formatHttpHeaderAttributes(array $attributes = []);

Drupal\Core\Ajax\AjaxResponseAttachmentsProcessor implements this interface but doesn't implement this method. As this is a static I don't think it needs to be on the interface, it can just be declared on the class only?

+++ b/core/includes/common.inc
@@ -356,15 +357,16 @@ function date_iso8601($date) {
+  @trigger_error("drupal_http_header_attributes() is deprecated nn Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Render\HtmlResponseAttachmentsProcessor::formatHttpHeaderAttributes() instead. See https://www.drupal.org/node/3000051.", E_USER_DEPRECATED);

makes sense to add test to make sure this error is triggered

Patch
- removes bethod from interface & marks @internal
- fix deprecated message
- adds test coverage

Looks good!

Committed f577bdc and pushed to 8.7.x. Thanks!

+++ b/core/includes/common.inc
@@ -350,21 +351,22 @@ function date_iso8601($date) {
+  @trigger_error("drupal_http_header_attributes() is deprecated nn Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Render\HtmlResponseAttachmentsProcessor::formatHttpHeaderAttributes() instead. See https://www.drupal.org/node/3000051", E_USER_DEPRECATED);

We made a typo here FYI - s/nn/in/

Fix typo

Patch was already in, but made the same change locally and committed it.

CR published