Code

#18660 closed Bug (fixed)

View on Site links get localized and break urls

Reported by: eduardocereto Owned by: nobody
Component: contrib.admin Version: 1.4
Severity: Normal Keywords: localization
Cc: eduardocereto@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

When object ids are larger than 1000 and you have localization turned on the "View on Site" links on the admin break because the ids get a thousand separator and that render invalid shortcut urls.

This is a very old bug and I'm sure I reported it before. Just can't seem to find it anymore. But I just confirmed it still happens on 1.4.

I know the fix should be simple. It's just a matter of using the template filter unlocalize or maybe {% localize off %} around the problematic code. But I fid this solution somehow not very elegant.

Possibly affected files:

admin/templates/admin/change_form.html
admin/templates/admin/edit_inline/stacked.html
admin/templates/admin/edit_inline/tabular.html

Attachments (0)

Change History (7)

comment:1 Changed 21 months ago by eduardocereto

  • Cc eduardocereto@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 21 months ago by eduardocereto

  • Has patch set

Pull Request with possible Fix

https://github.com/django/django/pull/226

comment:3 Changed 21 months ago by eduardocereto

  • Resolution set to invalid
  • Status changed from new to closed
  • Triage Stage changed from Unreviewed to Fixed on a branch

Seems like the master branch already addresses this in a different manner. I'm not sure how it works, but it does. So this change is unnecessary anymore. Sorry I should have tested the master branch before opening this ticket.

Fixed on #18433

Last edited 21 months ago by eduardocereto (previous) (diff)

comment:4 Changed 21 months ago by eduardocereto

I added a regression test for this bug.

https://github.com/django/django/pull/227

comment:5 Changed 20 months ago by aaugustin

  • Resolution invalid deleted
  • Status changed from closed to reopened

Re-opening to consider the addition of this test case.

comment:6 Changed 20 months ago by matthewwithanm

  • Triage Stage changed from Fixed on a branch to Ready for checkin

The test looks good; fails on commit 108f8dd, passes afterwards.

comment:7 Changed 19 months ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from reopened to closed

In c9c9a5642512155e9693bf5fa6221fd26adeccca:

Added complementary regression test for commit c1729510

Also fixed #18660.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.