Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The URL alias admin page at admin/build/path
offers Edit and Delete for aliases, but no equivalent to View, which would mean turning the displayed URL (alias or source) into a link.
The suggested patch adds that feature to the "System" column.
Comment | File | Size | Author |
---|---|---|---|
#33 | drupal.path-admin.patch | 2.06 KB | sun |
#31 | drupal.path-admin.patch | 1.93 KB | sun |
#30 | drupal.path-admin.patch | 1.92 KB | sun |
#30 | url-aliases-warning.png | 10.67 KB | sun |
#27 | drupal.path-admin-288039_1.patch | 1.07 KB | ff1 |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedThere is no reason to do the check_plain for the text since the l function[1] does that already.
[1] http://api.drupal.org/api/function/l/7
Comment #2
catch@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.
Comment #3
Damien Tournoud CreditAttribution: Damien Tournoud commentedAs 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.
Comment #4
catchWhoops sorry. Clearly I can't read :(
Comment #5
fgmChanged. I had left it because it was there already, but it is indeed superfluous.
Beyond this, should the link be on the canonical path, the alias, or both ? I currently added it only to the canonical path.
Comment #6
Anonymous (not verified) CreditAttribution: Anonymous commentedLooks good to me.
Comment #7
Damien Tournoud CreditAttribution: Damien Tournoud commentedIt doesn't matter: it will get canonicalized by url() anyway.
The patch looks good to me, too.
Comment #8
fgmI was asking from a usability perspective, of course: what is the best for the user of that page. I think it is the current choice (the second colum), but others might disagree (first column, or both).
Comment #9
Dries CreditAttribution: Dries commentedOn 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?
Comment #10
fgmNew patch based on Dries' suggestion, but with "visit" changed to "view", in order to stay in line with current practice in core.
This being said, I tend to find aliases being typically much longer than the ones in this example, meaning links are not usually as much spaced as in the screenshot, and adding one column for this operation seems to add some unneeded clutter. Maybe we should rather achieve a similar spacing reduction without by not forcing the table width to 100%, but then this would be theme-dependent.
Maybe at some point it would be a good idea to include a set of standard icons for standard operations throughout core, like View/Edit/Delete and Move Top/Move Up/Move Down/Move Bottom (à la Views UI) ?
Comment #11
catchfgm'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.
Comment #12
Anonymous (not verified) CreditAttribution: Anonymous commentedWhat Catch said. The link is enough to represent viewing the page and I too believe that Dries' Operations issue is more a theme issue.
Comment #13
Dries CreditAttribution: Dries commentedA 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.
Comment #14
fgmThe problem with URL truncation is twofold:
- we don't know when preparing the page how wide the actual display area is, since it depends on the theme layout, the font size, and the browser window size. We might end up uselessly truncating paths to unusable widths and leaving large empty areas for sysadmins with wide screens, while still not truncating enough for some users with small displays like an EeePC. It seems to me this is an issue which can only be handled by client-side JS, where this information is available. And as such, it seems to belong more in the theme than in path.module. Do we really want to add JS to this module ?
- I think we do not (yet) have a standard ellipsize/truncate library function, either in JS or PHP, like the one in Pango, for instance. If we want to start handling screen fitting of strings, we will need one, won't we ?
Comment #15
catchAlso - 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.
Comment #16
swentel CreditAttribution: swentel commentedIf locale module is enabled, another column is added with language, and the delete operation isn't renderend anymore after this patch.
Comment #17
swentel CreditAttribution: swentel commentedNew patch which fixes colspans when locale module is enabled.
Comment #18
lilou CreditAttribution: lilou commentedRe-roll.
Comment #19
meba CreditAttribution: meba commentedSeems OK to me. Still applies and works.
Comment #20
lilou CreditAttribution: lilou commentedRemove windows line breaks.
Comment #21
ultimateboy CreditAttribution: ultimateboy commentedTested patch. Appears to work without a hitch.
Attached updated screenshot to account for changes.
Comment #22
yoroy CreditAttribution: yoroy commentedThe '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.
Comment #23
agentrickardI 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.
Comment #24
swentel CreditAttribution: swentel commentedInitial patch didn't get in, so last patch misses some lines (especially when local is enabled)
Comment #25
agentrickardWhatever. I'm out.
Comment #26
sunComment #27
ff1 CreditAttribution: ff1 commentedWe 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.
Comment #28
alexanderpas CreditAttribution: alexanderpas commentedI would even say, remove the link from the system path... opinions?
Comment #29
sunIf 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?
Comment #30
sunOwww, nice! :)
Comment #31
sunOf course, this should take $data->language into account.
Comment #32
Dries CreditAttribution: Dries commentedPlease add a code comment explaining
+ 'class' => ($data->dst != drupal_get_path_alias($data->src, $data->language) ? 'warning' : NULL),
.Comment #33
sunAdded inline comment.
Comment #34
alexanderpas CreditAttribution: alexanderpas commentedlooks good! (btw: how about backporting this patch?)
Comment #35
swentel CreditAttribution: swentel commentedTestbot happy and tests also pass on postgres too, so ready to go !
Comment #36
Dave ReidVery nice addition.
Comment #37
Dries CreditAttribution: Dries commentedGlad we were able to drive this one home. Committed to CVS HEAD. Thanks to all people involved.
Comment #38
swentel CreditAttribution: swentel commented@Dries
when commiting this, #342294: Rename poll, profile and taxonomy modules tables to singular also slipped in, cf http://drupal.org/cvs?commit=157053
Comment #39
sunComment #41
alexanderpas CreditAttribution: alexanderpas commentedtestbot race condition? lol!
Comment #42
Dave ReidThis continues to not be corrected in HEAD after a day. :/
Comment #43
Dries CreditAttribution: Dries commentedIt was a mistake to let that other patch slip in, but it is probably OK now.
Comment #45
sun