Followup from #2343759: Provide an API function to replace url()/l() for external urls. Part of #2339219: [meta] Finalize URL generation API (naming, docs, deprecation).

Problem/Motivation

In a few places in #2343759: Provide an API function to replace url()/l() for external urls, we do this:

strpos($this->uri, 'base://') !== 0)

Proposed resolution

Add a method for this.

Remaining tasks

  • Decide what to name it.
  • Add it.
  • Convert things to it.

User interface changes

None.

API changes

API addition only.

Postponed until

#2343759: Provide an API function to replace url()/l() for external urls

CommentFileSizeAuthor
#6 provide_a_method_for-2346859-6.patch592 byteshussainweb
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

Status: Postponed » Active
kgoel’s picture

I have found only one occurrence of strpos($this->uri, 'base://') !== 0) in core/Url.php. Is this issue still relevant?

effulgentsia’s picture

Status: Active » Fixed

Provide a method

We have that. It's isExternal() :)

In a few places

Per #2, it's down to one place, which is necessary, since isExternal() needs some implementation.

Is this issue still relevant?

I don't think so. Marking fixed since the other places have been fixed via some other issues after this was originally opened.

effulgentsia’s picture

We have that. It's isExternal().

Correction: if you specifically want to know whether it's not routed and not external, you can call !$url->isRouted() && !$url->isExternal(). I don't think we need a single-method wrapper for that: those are two different checks so can remain distinct.

hussainweb’s picture

It seems like a good idea to remove the comment leading to this issue in \Drupal\Core\Url::setUnrouted(). Should we do it in this issue?

    // @todo Add a method for the check below in
    // https://www.drupal.org/node/2346859.
    if ($this->external = strpos($this->uri, 'base://') !== 0) {
      $this->options['external'] = TRUE;
    }
hussainweb’s picture

Status: Fixed » Needs review
FileSize
592 bytes

Just giving it a shot. I can't think of any other issue better suited to remove the comment.

hussainweb’s picture

Priority: Normal » Minor
Issue tags: +Quickfix

Marking it as a quickfix, and a minor issue, which it is. :)

xjm’s picture

Status: Needs review » Closed (duplicate)
Related issues: +#2416763: Convert Url::fromUri() base:// scheme to base:

This is actually going to end up as a duplicate of #2416763: Convert Url::fromUri() base:// scheme to base: now, which will refactor the lines with the @todo.