It looks like hook_url_outbound_alter() is no longer a thing. As far as I can tell the proper way to do this now would be to create a PathProcessor and alter URLs that way and that hook_url_outbound_alter() doesn't even exist anymore. So it should be removed.

grep -R url_outbound gives the following:

./core/includes/common.inc:          // url() generates the script and prefix using hook_url_outbound_alter().
./core/includes/common.inc:          // Instead of running the hook_url_outbound_alter() again here, extract
./core/lib/Drupal/Core/Path/AliasWhitelist.php:    // items such as paths that will be rewritten by hook_url_outbound_alter().
./core/modules/system/system.api.php:function hook_url_outbound_alter(&$path, &$options, $original_path) {

Which all appear to be just documentation references.

For reference:
https://api.drupal.org/api/drupal/core%21modules%21system%21system.api.p...

AliasWhitelist.php has this in it which should be updated but I'm not sure what it should be updated to.

\Drupal\Core\Path\AliasWhitelist

  public function get($offset) {
    $this->lazyLoadCache();
    // url() may be called with paths that are not represented by menu router
    // items such as paths that will be rewritten by hook_url_outbound_alter().
    // Therefore internally TRUE is used to indicate whitelisted paths. FALSE is
    // used to indicate paths that have already been checked but are not
    // whitelisted, and NULL indicates paths that have not been checked yet.
    if (isset($this->storage[$offset])) {
      if ($this->storage[$offset]) {
        return TRUE;
      }
    }
    elseif (array_key_exists($offset, $this->storage)) {
      return $this->resolveCacheMiss($offset);
    }
  }

And this is in common.inc, again, not sure what to do. But it seems like the comment may not even be relevant anymore?

          // url() generates the script and prefix using hook_url_outbound_alter().
          // Instead of running the hook_url_outbound_alter() again here, extract
          // them from url().
          // @todo Make this less hacky: http://drupal.org/node/1547376.

The code referenced in the todo issue (https://drupal.org/node/1547376) doesn't even exist anymore. So I'm guessing this comment can just be removed.

I think there are basically two tasks here.

1. Confirm that hook_url_outbound_alter is no longer a valid hook name.
2. Remove it from system.api.php and other documentation.

Comments

kokobutter’s picture

Assigned: Unassigned » kokobutter
kokobutter’s picture

Hi i think there is a mistake in your files.
The files still exist. on notepad++ it on line 2519 or on aptana it on line 2457.
Check your file again

kokobutter’s picture

Status: Active » Needs review
eojthebrave’s picture

Issue summary: View changes

That's correct. The code is still in system.api.php but I don't think it should be. That code is there for example/documentation purposes. However, it documents a hook that I don't think Drupal actually has anymore. hook_url_outbound_alter doesn't seem to ever get fired. And instead you need to create a PathProcessor to do what you would have done in hook_url_outbound_alter for Drupal 7.

I think there are basically two tasks here.

1. Confirm that hook_url_outbound_alter is no longer a valid hook name.
2. Remove it from system.api.php and other documentation.

eojthebrave’s picture

Status: Needs review » Needs work
jhodgdon’s picture

Title: Remove hook_url_outbound_alter() from system.api.php » Where did hook_url_outbound_alter go?
Priority: Normal » Critical
Status: Needs work » Active

I agree with your assessment that hook_url_outbound_alter() no longer exists (as in, it's a documented hook but it is not ever invoked).

However, there is no change notice mentioning this. That is a critical issue. Someone needs to figure out which issue removed invoking this, and write a change notice (or add something to an existing change notice so that someone can search for hook_url_outbound_alter at https://drupal.org/list-changes and find the change notice).

And we definitely do need to remove mention of this hook from all documentation. I have no idea about how to update the docs in the AliasWhitelist class or common.inc. Some comment is needed in both cases to explain what the code is doing, but I'm not sure what it should be, since the code is obscure (to me anyway).

kokobutter’s picture

Assigned: kokobutter » Unassigned
drifter’s picture

If I read this correctly, looks like it got removed from url() in this huge patch, and got replaced with OutboundPathProcessorInterface:

https://drupal.org/comment/7502544#comment-7502544

-  // Allow other modules to alter the outbound URL and options.
-  drupal_alter('url_outbound', $path, $options, $original_path);

on #1888424: Make Drupal's URL generation logic available to HttpKernel, and minimize code repetition/divergence. There is a change notice, but doesn't mention the hook:

https://drupal.org/node/2046643

jhodgdon’s picture

Good sleuthing -- thanks for finding that!

So someone should update that change notice so it mentions this hook, and maybe check the patch that was committed (or patches? There seem to be a couple of issues mentioned in that change record), and make sure that there are not any other hooks that were left out of the docs.

And we would also need a code sample for how to modify D7 code that used this hook (and any others) for how to do the equivalent in 8.x.

tim.plunkett’s picture

Category: Bug report » Task
Priority: Critical » Major
Issue tags: +Missing change record, +Needs change record

Tagging appropriately

xjm’s picture

Priority: Major » Critical
Issue tags: +beta blocker

Explicitly tagging missing change records as beta blockers.

Gaelan’s picture

Assigned: Unassigned » Gaelan

Working on this.

Gaelan’s picture

Assigned: Gaelan » Unassigned
Status: Active » Needs review

Added a change notice.

gábor hojtsy’s picture

Status: Needs review » Fixed

Reviewed, published. Fixed some minor things, nothing big: https://drupal.org/node/2238759/revisions/view/7128849/7128895

Gaelan’s picture

Status: Fixed » Active
Issue tags: -Missing change record, -Needs change record

We also need to remove this from the api.php. One sec.

Gaelan’s picture

Status: Active » Needs review
StatusFileSize
new1.38 KB

A sec has passed.

gábor hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Looks good. Simple.

xjm’s picture

Change record looks great. Thanks for finding the stale docs as well.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

The patch is totally correct, but replaces documentation on how to do some set of tasks with Drupal with... absolutely nothing. :( The change notice exists, and is great, but only will work for D7 developers. #2238935: [meta] Complete missing documentation for special strings and metadata like annotation keys, routing parameters, tagged services, etc. is a spin-off issue to discuss this larger problem, which obviously wasn't introduced here.

Committed and pushed to 8.x. Thanks!

  • Commit 88f026e on 8.x by webchick:
    Issue #2228795 by Gaelan | eojthebrave: Where did...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.