URL alias admin page should include a link to the aliased URL

@Earnie: the check_plain is completely outside the l() function and not introduced by the patch.

I can't believe this feature was never there before. This is lovely. Tested works. RTBC.

As far as I can see, Earnie is right, there is a superfluous check_plain() that should be removed because already in the added l() call.

Whoops sorry. Clearly I can't read :(

Beyond this, should the link be on the canonical path, the alias, or both ? I currently added it only to the canonical path.

It doesn't matter: it will get canonicalized by url() anyway.

The patch looks good to me, too.

On my screen, the Operations column is not visually pleasing -- it is too spread out (see screenshot). Adding a 'visit'-link might make it look better and might make it more explicit and thus easier to spot?

fgm's right - I've had url alias tables break some themes with long aliases (i.e. bleeding into the right sidebar and non-clickable ops). So I'd prefer #5 to conserve space for those situations, and because the width of the table doesn't have much to do with this patch IMO, more a Garland issue.

A possible solution for the long URLs is to truncate them as we usually do. As it is related to this patch, it would be good if we could tack this onto the current patch. It would probably be handy to see screenshots of both approaches with a real-world alias table.

Also - if I have a url like example.com/foo/bar/baz/123 and example.com/foo/bar/baz/1234 and they both get truncated to example.com/foo/bar/baz... then that screen becomes a lot less useful for administering paths IMO.

If locale module is enabled, another column is added with language, and the delete operation isn't renderend anymore after this patch.

New patch which fixes colspans when locale module is enabled.

Re-roll.

Seems OK to me. Still applies and works.

Remove windows line breaks.

Tested patch. Appears to work without a hitch.

Attached updated screenshot to account for changes.

The 'view' operation is inconsistent with how similar screens in core do this. They link the title itself for viewing.

Tracker:

Content listing:

Comments:

I'd prefer doing same here.

I agree with yoroy.

One-line patch attached. Turns both the alias and the src into links -- both of which should point to the aliased path.

Initial patch didn't get in, so last patch misses some lines (especially when local is enabled)

Whatever. I'm out.

We need to also think about new users with this patch. I know that when I created my first url alias, I wanted to test it out the make sure it worked, so I think it would be useful if both paths were linked.

The attached patch adds the link to both the system path and the alias.

I would even say, remove the link from the system path... opinions?

If we link both, we can add a sanity check here: The alias link should _show_ the alias, but use src (not dst) as target path. So, by hovering over the link you can immediately see whether Drupal thinks that the internal path really translates into the displayed alias.

Oww... couldn't we even add a CSS class then to highlight all aliases that are obsolete?

Owww, nice! :)

Of course, this should take $data->language into account.

Please add a code comment explaining + 'class' => ($data->dst != drupal_get_path_alias($data->src, $data->language) ? 'warning' : NULL),.

Added inline comment.

looks good! (btw: how about backporting this patch?)

Testbot happy and tests also pass on postgres too, so ready to go !

Very nice addition.

Glad we were able to drive this one home. Committed to CVS HEAD. Thanks to all people involved.

@Dries
when commiting this, #342294: Rename poll, profile and taxonomy modules tables to singular also slipped in, cf http://drupal.org/cvs?commit=157053

testbot race condition? lol!

This continues to not be corrected in HEAD after a day. :/

It was a mistake to let that other patch slip in, but it is probably OK now.