Opened 17 years ago

Closed 14 years ago

Last modified 14 years ago

#3695 closed (fixed)

duplicate html id's in doc pages

Reported by: Simon G. <dev@…> Owned by: simeon
Component: contrib.admin Version: dev
Severity: Keywords: admin, docs, xhtml
Cc: dev@…, sf@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Whilst trying to fix #2411 (Validation errors in template filters/tags documentation) I came across this minor issue. The templates loop over each tag/filter, and title them with something like <h3 id="{{ tag.name|escape }}">{{ tag.name|escape }}</h3>.

When there are two template tags named the same, then there will be an id clash, causing an (x)html validation error (and the anchor tags won't work properly for the two duplicate id's):

# Error  Line 525 column 12: ID "filter" already defined.

    <h3 id="filter">filter</h3>

An "id" is a unique identifier. Each time this attribute is used in a document it must have a different value. If you are using this attribute as a hook for style sheets it may be more appropriate to use classes (which group elements) than id (which are used to identify exactly one element).

Currently, this is only an issue in template_tag_index.html, where we have a filter tag in the defaulttags section and in the admin_list section.

It can be fixed by prepending the library name to the id, so that each one will be unique. I'll add a patch once I've finished up #2411

Attachments (2)

3695.diff (1.3 KB ) - added by simeon 16 years ago.
3695.2.diff (2.6 KB ) - added by simeon 14 years ago.
Updated to apply against r13096

Download all attachments as: .zip

Change History (18)

comment:1 by Chris Beaven, 17 years ago

Triage Stage: UnreviewedAccepted

comment:2 by simeon, 16 years ago

Owner: changed from nobody to simeon
Status: newassigned

comment:3 by simeon, 16 years ago

Has patch: set

Ok, just to be clear on how to replicate the bug and verify that it is fixed:

Start a Django project with admin. Login to the Admin area and click on the "documentation" link and then on the "Tags" link to load the doc page for template tags.

On the left the index lists the tags for various modules with links that jump to the relevant tag based on ID. This works as long at there are no duplicate tag names - the admin_list module, however, defines a "filter" tag that then uses the same ID as the built in filter tag. Clicking on either link entitled filter in the index brings you to the built in filter tag (and as mentioned the page is now not valid HTML).

I applied the fix Simon G recommended by prefixing the ID and link target with the relevant module name (or built_in for the built in filters). See 3695.diff. Upon applying navigation ought to work properly and ID/link target elements should be inspected (yea firebug) to verify that they are constructed properly.

by simeon, 16 years ago

Attachment: 3695.diff added

comment:4 by simeon, 16 years ago

Cc: sf@… added

comment:5 by Matt McClanahan, 16 years ago

milestone: 1.0

comment:6 by Malcolm Tredinnick, 16 years ago

milestone: 1.0

As best I understand it, this doesn't create a display problem, it doesn't cause Django to crash, it doesn't prevent anything from actually working. It's a slight annoyance in some edge-cases creating invalid output at the markup level on the documentation page (only). If that's all correct, it can wait until after 1.0.

comment:7 by Matt McClanahan, 16 years ago

I'll grant it's pretty minor, but there is one thing that doesn't work: linking to a particular tag/filter when more than one exists in the page. In most browsers, links will always go to the first occurrence of the id.

comment:8 by Adam Nelson, 14 years ago

Patch needs improvement: set

All of admin_doc has been refactored - either this is no longer an issue or at the least the patch needs to be redone.

comment:9 by Adam Nelson, 14 years ago

Actually, I was looking at the wrong admin_doc directory (Which should be deleted). The patch still needs to be updated though to apply to the new location of admin_doc.

by simeon, 14 years ago

Attachment: 3695.2.diff added

Updated to apply against r13096

comment:10 by simeon, 14 years ago

Ok - I updated the patch to apply to latest trunk - I also added the same fix to the filters template. However in testing it I noticed that in current trunk built-in filters and tags are being shown, only tags and filters that belong to modules. I'll open (or find an existing) ticket for this...

comment:11 by Adam Nelson, 14 years ago

Triage Stage: AcceptedReady for checkin

comment:12 by Adam Nelson, 14 years ago

Patch needs improvement: unset

comment:13 by Malcolm Tredinnick, 14 years ago

Resolution: fixed
Status: assignedclosed

(In [13728]) Fixed id attribute generation in the admin docs page. Patch from simeon.

Fixed #3695. Thanks.

comment:14 by Karen Tracey, 14 years ago

(In [13737]) Adjust AdminDocTests to run after r13728. Also match comments to tests and add test that was there in comment form only.Refs #3695.

comment:15 by Malcolm Tredinnick, 14 years ago

(In [13747]) [1.2.X] Fixed id attribute generation in the admin docs page. Patch from simeon.

Fixed #3695. Thanks.

Backport of r13728 from trunk.

comment:16 by Malcolm Tredinnick, 14 years ago

(In [13750]) [1.2.X] Adjust AdminDocTests to run after r13728. Also match comments to tests and add test that was there in comment form only.Refs #3695.

Backport of r13737 from trunk.

Note: See TracTickets for help on using tickets.
Back to Top