This is a direct offspring of the help patch. The code is taken from there but changed the prefix to internal: and the strtr to str_replace. Once this is in we can use this in the help files and everywhere else and we get caching for free.

Comments

chx’s picture

StatusFileSize
new2.73 KB
chx’s picture

StatusFileSize
new2.67 KB

First I uploaded the wrong patch, then left in debug. now...

chx’s picture

StatusFileSize
new2.66 KB

DamZ does not like node/123. Sure, I can copy the foo/bar from the test.

chx’s picture

StatusFileSize
new3.32 KB

Now, I forgot to list the poor thing. No wonder, given that op 'list' was not properly formatted.

damien tournoud’s picture

Status: Needs review » Reviewed & tested by the community

Code looks nice. This is the result on the node edit form if you add the filter to the default text format:

Only local images are allowed.

damien tournoud’s picture

StatusFileSize
new3.63 KB

Now with a less confusing help tip.

Only local images are allowed.

john morahan’s picture

Status: Reviewed & tested by the community » Needs work

yay, pathfilter in core :) but...
1. how is url('$1') supposed to find a path alias for anything other than literally '$1'?
2. maybe an edge case given they will usually be individually quoted, but the preg_replace does a greedy match so "[internal:node/1] [internal:node/2]" won't work as you expect.

damien tournoud’s picture

Status: Needs work » Needs review
StatusFileSize
new3.63 KB

This should take care about John's concerns:

- Extended the test
- Fixed the filter (we really need to use the /e modifier, because we need to properly alias paths).

damien tournoud’s picture

Status: Needs work » Needs review
StatusFileSize
new3.48 KB

And don't forget to remove old cruft ;)

webchick’s picture

Status: Needs review » Needs work

Comments from #drupal:
* testInternalFilter has no PHPDoc.
* if we're going to call _filter_internal() from the test, should we drop the leading underscore?
* $f is not an acceptable variable name.
* also, what is _filter_internal_callback? Looks unused.

webchick’s picture

...was what I said about #8! ;)

webchick’s picture

AHEM. code needs review?

damien tournoud’s picture

StatusFileSize
new3.53 KB

And from #10. The test is an unit test, so it's logical it tests internal functions ;)

damien tournoud’s picture

StatusFileSize
new3.59 KB

Reworked the wording of the description with catch via #drupal.

catch’s picture

Status: Needs review » Reviewed & tested by the community
add1sun’s picture

So, after talking with webchick on IRC and getting the rundown I'll +1 this. I prefer the internal: term over base_url and I really like the fact that the syntax can be used in other places in core rather than being "just" a help system thingy. That will make explanation and use more consistent for folks.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Ok, then!

Added a CHANGELOG.txt entry and committed. Thanks, folks!

Bojhan’s picture

Just tagging, so I can keep track.

alexanderpas’s picture

Could we include a [files:] filter too? and automatic conversion? a.k.a. can we get a fully fixed PathFilter in core?

shameless plug: #402296: Call for Action: Get Pathfilter Additional Functionality in before D7 makes pathfilter obsolete (Pathfilter in Core???)

webchick’s picture

We talked about doing automatic conversion of any link that starts with "/" to use internal. This is problematic for the following reasons:

a) Sites may be using other programs apart from Drupal on their domains (amazing, but true! ;)). For example, you would *not* want a link to "/phpMyAdmin" to get translated to http://www.example.com/drupal/phpMyAdmin.
b) Without a special token that can be regexed against, we now need to check *every* a, img, link, style, script, and who knows what other tags. That seems like a big performance drain.

As for [files], I'm not really opposed. But we don't (afaik) need it for help, which was the point of committing this patch. So if you want to champion for that, it should be done in another issue.

sun’s picture

Issue tags: +Hall of shame

Until now, Drupal core did not implement inline macro tags. Now it does. And it introduces (and thereby teaches) a syntax that is known to be inflexible.

Furthermore, did anyone test whether this works with any client-side editor? ...

webchick’s picture

Status: Fixed » Needs work

Sun: could you elaborate on your concerns?

sun’s picture

Status: Needs work » Fixed

Recommended reading: #80170: Inline API (dates back to 2006/Drupal 4.7)

Specifically about syntax: #32838: New Inline Filter code

Why is this inflexible? Because the syntax is known to conflict with macro arguments.

What would be a proper macro syntax? [link|path=admin/user/permissions|title=Administer permissions|absolute=0|fragment=my-anchor]

What would be an alternative syntax? url:///admin/user/permissions or similar. No square brackets.

Testing what has been added to core requires to install Wysiwyg API with all supported editors, use the "Link" button of each editor and enter this macro tag. Afterwards, disable the editor and ensure that a) the square brackets are not escaped b) the entire href attribute value does not have a leading slash or any other addition or alteration.

Also:

Why is the filter and function called "internal" and not "url"?

Why is a filter tag called "internal" not intended to display a piece of content for internal users? What has "internal" in common with links and URLs? Ah, yes, Drupalitis.

Since when does url() support only internal paths?

Why is this a private function?

What happens if the user disables this filter?

What happens if users "suddenly" start to use this macro in their content and we suddenly decide to rename it in Drupal 8?

...

Anyway. Let's just continue to break each and everything.

chx’s picture

Status: Fixed » Needs work

Please do NOT re-close this. Your concerns are valuable and needs discussion.

chx’s picture

Well, then we need something more sophisticated, I agreed with sun that a) he stops using macro because that leads to an endless bikeshedding over what the fuck is a macro b) we remove the [ ] and just let <a href="internal:foo/bar"> happen.

chx’s picture

Status: Needs work » Reviewed & tested by the community
StatusFileSize
new3.28 KB

Please revert this and let others get in inline API or token (which is already beyond saving -- I tried and was chased from the issue) in core.

sun’s picture

Status: Reviewed & tested by the community » Needs work

+1

sun’s picture

Status: Needs work » Reviewed & tested by the community
tstoeckler’s picture

Status: Reviewed & tested by the community » Fixed

@sun: From your point of view, would there be a way to turn this "shameful" patch into a not so shameful one, e.g. by replacing the 'internal' tag by

[link|path=admin/user/permissions|title=Administer permissions|absolute=0|fragment=my-anchor]

or would that have to be a sweeping change as in e.g. moving Inline API to core?

keith.smith’s picture

Status: Fixed » Reviewed & tested by the community

I don't think that last comment meant to change the status.

webchick’s picture

Status: Reviewed & tested by the community » Active

Rolled back. I agree this needs more discussion. From sun on IRC:

"There is a difference between auto-formatting filters and macro filters. The former is what we have in core thus far: Filters that don't require you to enter something special, they just convert content based on standards (not defined by ourselves). Macro filters are different: The require a very certain syntax, most of them can take parameters, and when their syntax changes, all content that used them in the past *are broken*. My goal to solve the latter is to get Inline API into a core-worthy state, which would (also) handle inline tag /upgrades/."

So essentially, we're introducing a special syntax here, and if we ever change our minds that's a whole lot of hassle.

Tokens in core is likely the way to go. Then this would be one standard way of introducing "special things" everywhere. sun, feel like un-sticking another issue? ;)

I'm also curious to know more about inline API and how that would work. Is that basically to allow each site to define its own inline styles?

Bojhan’s picture

Removing tags to avoid confusion,

chx’s picture

Assigned: chx » Unassigned
mki’s picture

We should probably extend Extensible HyperText Markup Language. So use standard XML syntax (< > instead of [ ] brackets, tags, parameters). These are the benefits:
1. Set of powerful and efficient PHP functions for XML, XHTML, DOM processing. Some of them are part of the PHP core and are enabled by default. See http://php.net/manual/en/refs.xml.php
2. Standard escape entities for < > brackets.
3. No need to learn completely new syntax. Standard parameters available.

For new tags we should probably use prefix "drupal:" for Drupal namespace, for example:

<drupal:title /> <!-- Display node title in body text -->
<drupal:authorName /> <!-- Display user's name -->

For internal links we should probably "extend" Uniform Resource Locator with new scheme, for example:

<a href="drupal://node/1" />
<a href="internal://node/1" />

More examples: http://en.wikipedia.org/wiki/URI_scheme#Official_IANA-registered_schemes

Would be great if we also consider connection with Node reference and User reference modules and RDF too.

sun’s picture

@mki: Sorry, but a big no to any DTD customizations. Client-side editors (aka. WYSIWYG) don't support that and will simply strip anything unknown from the content.

We don't need to invent a new URL scheme, because if we go with a custom scheme, then the URLs will be converted to a standard scheme by the filter before the content is displayed.

However, also a custom URL scheme would have to be tested for compatibility with client-side editors first.

akahn’s picture

StatusFileSize
new752 bytes

Here's a patch, since we need to remove this from CHANGELOG.txt if it is not going into Drupal 7.0.

akahn’s picture

StatusFileSize
new825 bytes

Ignore that last patch. ;)

chx’s picture

Status: Active » Reviewed & tested by the community

Please commit and reset to active.

webchick’s picture

Status: Reviewed & tested by the community » Active

D'oh! Thanks. Committed the reverting of CHANGELOG.txt.

alan d.’s picture

Just as an aside, have the main search engines fixed their issues with using cached pages BASE href's? The _base_href attribute used on cached Google pages seems to work fine. If so, are there any outstanding issues now with using the BASE header tag and relative URL's? Yep, back to the good old days of Drupal 4.6!

We've used the BASE tag on a couple Drupal 6 sites recently due to the issue of getting the users to correctly use the internal: tag on links and we have yet to find any issues.

Garrett Albright’s picture

b) Without a special token that can be regexed against, we now need to check *every* a, img, link, style, script, and who knows what other tags. That seems like a big performance drain.

We're talking about doing this as a filter whose output will be cached, right? If so, then it only has to be done when the node is edited. No biggie, right?

I too am not a big fan of the idea of introducing a special syntax or prefix of some sort to paths to get this to work. As my module Pathologic demonstrates, it's possible to determine the user's intent with just some fairly simple heuristics and maybe a file_exists() (yes, I know, but again, it only has to happen once). Check out the code, or if the admittedly undercommented regex strings make your mind boggle, check out the "Is configuration necessary?" section of the docs page for a plain-English explanation of how it works.

This feature in core? A good but perhaps unnecessary idea. Introducing nonstandard markup in order to do it? No thanks!

(But if it has to be done with a tag of some sort, may I suggest a comment-style tag like <!--internal-->? It should break fewer things and has precedent (quality of that precedent aside) in the form of <!--break-->.)

sun’s picture

@Garrett Albright: Please ping me when you are in IRC. We need to talk.

mrfelton’s picture

I've recently taken over project ownership of Path Filter (which uses the internal:node/nid syntax) and am keen to see this functionality make its way into core. Work is also taking place to extend Path Filter to support a files: syntax, which was also mentioned earlier in this thread (#127484)

I have also begun communicating with Garrett Albright about the possibility of merging Path Filter and Pathologic, for obvious reasons - they do essentially the same thing, they just go about it in a slightly different way (and Pathologic actually does a lot more). With almost 1000 people using each of these modules and a push to get this functionality into core, if we are to merge these modules (and possibly if we don't too) then it would make sense that the module(s) were headed in the same direction this core patch - if indeed this core patch is going anywhere!

I am in two minds about weather or not special syntax should be applied ([] <> internal: etc.). Obviously, this is the way Path Filter works, and I'm used to this, but that certainly doesn't make it right.

Anyway... just though I'dd add myself to the discussion.

mrfelton’s picture

For reference, here is another discussion from the PathFilter issue queue, on the issue of using internal: (or another prefix) or not: #109992: Replace regular internal paths with URL aliases

dmitrig01’s picture

Let's solve this issue: we have proposals for:
1. internal://node/123
2. wiki - like syntax. [link:node/123] or [link:node/123|title]

let the discussions begin.

chx’s picture

I think we pretty much agree on that internal:/// is the least intrusive.

naheemsays’s picture

On irc an example of "internal:///drupal.org/node/401956#comment-1803458" was used and to me, I see no difference in using such a link from using "http://drupal.org/node/401956#comment-1803458" except that end users will know how to use the latter by default.

sun’s picture

Most probably, a solution like http://drupal.org/project/pathologic will be the best way to move forward. I already started with a #462870: Code clean-up, but the maintainer seems to be unresponsive.

Garrett Albright’s picture

It's true that the maintainer (me) hasn't worked on it in a while, but the dev release does include the patch you submitted. :P But your bump plus a support mail question has got me taking another look at the code tonight - perhaps I'll be making another beta release tonight.

I think we pretty much agree on that internal:/// is the least intrusive.

I think that no prefix at all is the least intrusive!

I'll go hang out in #drupal tonight if anyone feels the urge to ping me about this.

sun’s picture

Version: 7.x-dev » 8.x-dev
mfer’s picture

Can we get input from UX people or trainers with experience in teaching people? There are technical hurdles to overcome but one of the biggest is for everyday users. Non-technical content contributors. If non-technical users are inputing internal://node/123 or [link:node/123|title] which will be the easiest for them?

jose reyero’s picture

As @webchick suggested in #31

> Tokens in core is likely the way to go....

Not exactly the same, but related, using tokens to replace paths, links #1445144: Build better tokens with parameters (and use them to replace help text)

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • webchick committed 741a2e9 on 8.3.x
    #401956 by chx and Damien Tournoud: Add an optional filter to turn [...
  • webchick committed 59e5caa on 8.3.x
    Roll-back of #401956, which needs more discussion.
    
    
  • webchick committed 097f493 on 8.3.x
    #401956 follow-up by akahn: Forgot to remove from CHANGELOG.txt.
    
    

  • webchick committed 741a2e9 on 8.3.x
    #401956 by chx and Damien Tournoud: Add an optional filter to turn [...
  • webchick committed 59e5caa on 8.3.x
    Roll-back of #401956, which needs more discussion.
    
    
  • webchick committed 097f493 on 8.3.x
    #401956 follow-up by akahn: Forgot to remove from CHANGELOG.txt.
    
    

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • webchick committed 741a2e9 on 8.4.x
    #401956 by chx and Damien Tournoud: Add an optional filter to turn [...
  • webchick committed 59e5caa on 8.4.x
    Roll-back of #401956, which needs more discussion.
    
    
  • webchick committed 097f493 on 8.4.x
    #401956 follow-up by akahn: Forgot to remove from CHANGELOG.txt.
    
    

  • webchick committed 741a2e9 on 8.4.x
    #401956 by chx and Damien Tournoud: Add an optional filter to turn [...
  • webchick committed 59e5caa on 8.4.x
    Roll-back of #401956, which needs more discussion.
    
    
  • webchick committed 097f493 on 8.4.x
    #401956 follow-up by akahn: Forgot to remove from CHANGELOG.txt.
    
    

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

  • webchick committed 741a2e9 on 9.1.x
    #401956 by chx and Damien Tournoud: Add an optional filter to turn [...
  • webchick committed 59e5caa on 9.1.x
    Roll-back of #401956, which needs more discussion.
    
    
  • webchick committed 097f493 on 9.1.x
    #401956 follow-up by akahn: Forgot to remove from CHANGELOG.txt.
    
    

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.