UI to add arguments, query and hash to the linked path (patch)

I've been thinking a bit more about this, and it seems to me that the ability to pass additional arguments to the target item would be a valuable addition to more than just contact forms. It would be nice to be able to pass arguments to views page displays, for example.

So my thinking is now that I should simply add a field to the ckeditor link popup, "Extra arguments". This would be url-sanitized and appended to the link generated by ckeditor_link. The user should be able to enter additional arguments in path format or in query format, ie:

contact/marketing/competition (for use with views or the contact_forms module)
contact?category=marketing&subject=competition (for use with custom implementations)

See also #912220: Allow to add anchor to a Drupal link...

Cough ... some time later. I'm taking a look at this now, initially by just patching the module to provide a single additional field.

Handling appended query parameters and/or hash fragments doesn't look like it will be a problem, though security will need to be considered.

The difficulty with additional arguments is that the only thing that actually gets stored is the link url. Then the module has to parse that on plugin load to determine which link type to display, and reconstruct the human-readable title. There is no good way for the plugin to determine whether a url like /foo/bar/arg1/arg2 is a valid Drupal path as-is, or a valid path with additional arguments. The plugin would need to issue an additional ajax call to have the server load the menu router item and check.

The simplest way to approach this might simply be to stick with what we have, but allow the displayed field value to accept additional typed-in data, like:

Some node title (node/123/arg1/arg2)
Some node title (node/123#foo)
Some node title (node/123?param=value&something=false)

It's a little trickier for users because they have to add the extra data before the closing parenthesis, but it would really help avoid the problem of how to distinguish path arguments from user-added arguments when reverting the path.

I don't think it makes a great deal of sense to implement this as an add-on module. It turns out that very few changes are required to make this work, whereas an additional module would have to duplicate and maintain large chunks of code.

This patch allows the addition of query parameters and hash fragments to the displayed node and taxonomy paths (but not extra arguments, so you can't link to node/123/edit or taxonomy/term/456/edit), and the addition of extra arguments, query parameters and hash fragments to the displayed menu paths (so you can use views page arguments or exposed filter parameters).

And patch, duh.

Contrary to what I say above, it's worth noting that even without the patch, there is nothing stopping you from appending additional arguments to the path while in the dialog - the link will validate and be saved, and will work as expected. But when the dialog is reloaded, the extra arguments prevent switching to the "internal path" display, so you get "external url".

I think perhaps it is worth fully allowing extra arguments for node and taxonomy paths anyway, since it is already possible to link to things like node/123/edit? Then at least on reload, you'd get the expected internal path display, not external url. That's one less WTF for users. Opinions?

This patch includes the patch at #8, but also allows any extra arguments added to node and taxonomy term links to display correctly as "internal url" on reload.

And one more. This patch provides an extra field for entry of arguments, query parameters and hash fragment (or any combination). This considerably improves the discoverability and usability.

The primary ckeditor_link_revert() callback has been modified to load the router item from the supplied path, and returns that without any additional page arguments. This is compared to the full path on the client side, allowing the plugin to determine what the extra arguments are (if any).

Hash fragment

Query parameters

Views argument

All at once

That patch at #10 results in a trailing slash on the link URL if there are no additional arguments. This will prevent the ckeditor_link text filter from converting paths to aliases on output.

This updated patch adds an additional test for empty extra arguments before the plugin inserts the link in content.commit().

... and another patch. This improves the ckeditor_link_path_strip_args() function by comparing the number of path arguments to the number of router item path arguments. It handles things like Views menu items more reliably.

This patch also allows the ckeditor_link filter to process node and term paths with extra arguments, supporting the (optional) use of the Sub-pathauto module.

This patch fixes an illegal offset notice introduced in #12.

It also adds a setting at admin/config/content/ckeditor_link, "Extra arguments field", which provides a way to globally enable/disable the extra field in the dialog. The default is OFF, which preserves the standard ckeditor_link behavior.

I believe this is now almost ready to fly - it would be nice to get some feedback...

Anyone care to review this? I've had it in production for 6 months now with users reporting no issues.

@bereddeboer, which patch? #13?

My intent was to provide a full UI to let users add additional path arguments, query parameters and hash fragments, but @Devin Carlson seems to have hijacked the thread a little. The patch at #16 just allows other code to call the ckeditor_link_url() with an options array, and should either have been a separate issue, or supplied as a re-roll of the patch at #13.

Thanks - so can we perhaps drag this thread back to my original intent, providing a UI so users can pass arguments etc? The patch at #13 is the one for review and has been in use on 6 of my sites for a year. Hiding the one at #16.

Reroll against latest dev (just removes offsets).

Anyone remotely interested in reviewing this?

@bbu23 - use the "follow" button at the top of posts.

If you need this feature it would be useful if you could apply the patch, and if it works for you, mark the issue as "reviewed and tested by the community". I can't do that because it is my patch.

Thanks for the re-roll, I've had this in use for so long now that I forget it isn't actually in the module.

Feedback I have had from clients indicates that it might be desirable to have separate argument, query and fragment fields, but that could perhaps be a follow-up improvement if this gets in.

@anikrun - we have RTBC, how about a commit?

Patch re-rolled for latest dev. No changes.

Oops, dropped a line. Use this one.

Ahem. Let's remove the debugger statement in plugin.js.

Grr. This one.