Closed
Bug 546340
Opened 14 years ago
Closed 14 years ago
Change <a name> anchors to use @id
Categories
(Bugzilla :: Bugzilla-General, enhancement, P1)
Bugzilla
Bugzilla-General
Tracking
()
RESOLVED
FIXED
Bugzilla 3.6
People
(Reporter: GPHemsley, Assigned: GPHemsley)
References
Details
Attachments
(2 files, 2 obsolete files)
13.34 KB,
patch
|
mkanat
:
review+
|
Details | Diff | Splinter Review |
32.24 KB,
patch
|
mkanat
:
review+
|
Details | Diff | Splinter Review |
(This is the offshoot bug from bug 529201 comment 7 that neither I nor Max got around to filing until now.) Using <a name> wrappers to specify anchors has long been deprecated behavior and, as noted in bug 529201 comment 6, support for using @id for this purpose goes all the way back to IE5 (but not NN4). Thus, I think it is time that these obsolete tags are removed and replaced with the more modern usage of @id. For one thing, it would help prevent problems such as that of bug 546338.
Comment 1•14 years ago
|
||
Yes, agreed, if @id works then there's no reason to use <a name>. (This won't happen for 3.6, though, which is feature-frozen, and until somebody takes assignment of this bug, then we won't TM it.)
Severity: trivial → enhancement
Priority: -- → P1
Target Milestone: Bugzilla 3.6 → ---
Assignee | ||
Comment 2•14 years ago
|
||
This patch touches 17 template files, changing instances of <a name> to use @id instead. I've modified the surrounding markup in some places to have things make a little more sense (e.g. using <h3> instead of <b> or <p><b>). This patch does not touch any other elements that use @name, since they have a valid usecase on form elements. This patch also doesn't touch the release notes template, as that has 99 lines worth of <a name> instances. I'll have a separate patch for that later.
Comment 3•14 years ago
|
||
Comment on attachment 427028 [details] [diff] [review] Change <a name> to @id (excluding release notes) >=== modified file 'template/en/default/bug/edit.html.tmpl' >- <table class="status" cellspacing="0" cellpadding="0"> >+ <table id="bug_status_bottom" >+ class="status" cellspacing="0" cellpadding="0"> Nit: Our more normal spacing is like: <table id= class= >=== modified file 'template/en/default/bug/votes/list-for-user.html.tmpl' This template is now extensions/Voting/template/en/default/pages/voting/user.html.tmpl. >=== modified file 'template/en/default/pages/fields.html.tmpl' This file is awaiting an enormous patch in bug 529201 that's just about ready, so you might want to base your work on that patch instead. >=== modified file 'template/en/default/pages/quicksearch.html.tmpl' >=== modified file 'template/en/default/reports/components.html.tmpl' >+ <tr id="[% comp.name FILTER html %]"> You know, come to think of it, this doesn't have to be a valid id--it can start with a number or anything imaginable. Also, it can have just about *any* character in it. >=== modified file 'template/en/default/reports/duplicates.html.tmpl' >+<h3 id="explanation"> >+ What are "Most Frequently Reported [% terms.Bugs %]"? > </b> Cool. But </b> -> </h3>.
Attachment #427028 -
Flags: review? → review-
Assignee | ||
Comment 4•14 years ago
|
||
Hopefully, this addresses your concerns. It was applied after attachment 425729 [details] [diff] [review]. Otherwise, same conditions apply as before.
Attachment #427028 -
Attachment is obsolete: true
Attachment #427042 -
Flags: review?
Updated•14 years ago
|
Attachment #427042 -
Flags: review? → review-
Comment 5•14 years ago
|
||
Comment on attachment 427042 [details] [diff] [review] Change <a name> to @id (excluding release notes), v2 The only thing that's fixed in this patch is the spacing, and everything else seems to be the same.
Comment 6•14 years ago
|
||
By the way, you'll want to request review from specific people, or there's a chance that your review request won't be gotten to. See our development process here for details: http://wiki.mozilla.org/Bugzilla:Developers
Assignee | ||
Comment 7•14 years ago
|
||
Forgot to pull the latest revisions when I generated the last patch. This one should do it.
Attachment #427042 -
Attachment is obsolete: true
Attachment #427067 -
Flags: review?(mkanat)
Comment 8•14 years ago
|
||
Comment on attachment 427067 [details] [diff] [review] Change <a name> to @id (excluding release notes), v3 Okay, this looks good! :-)
Attachment #427067 -
Flags: review?(mkanat) → review+
Updated•14 years ago
|
Flags: approval+
Whiteboard: [waiting on checkin of blocker before being checked in]
Target Milestone: --- → Bugzilla 3.8
Assignee | ||
Comment 9•14 years ago
|
||
Separate patch for release notes, since there were 99 lines of change. This was done mostly via a couple of regexps.
Attachment #427074 -
Flags: review?(mkanat)
Comment 10•14 years ago
|
||
won't this generate invalid id's? eg. css id's can't start with a number, however a filename can start with a number.. <table id="[% file.filename FILTER html %] see http://www.w3.org/TR/CSS2/syndata.html#tokenization
Assignee | ||
Comment 11•14 years ago
|
||
(In reply to comment #10) > won't this generate invalid id's? Yes, it will. But the existing code already does, too. So we decided we'll address that issue in another bug, after this one has been applied.
Comment 12•14 years ago
|
||
Comment on attachment 427074 [details] [diff] [review] Change <a name> to @id in release notes Sweet! :-) Looks good. :-)
Attachment #427074 -
Flags: review?(mkanat) → review+
Comment 13•14 years ago
|
||
Thanks for the patch, gphemsley! :-) Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/ modified template/en/default/account/auth/login.html.tmpl modified template/en/default/account/prefs/email.html.tmpl modified template/en/default/admin/params/common.html.tmpl modified template/en/default/attachment/diff-file.html.tmpl modified template/en/default/attachment/list.html.tmpl modified template/en/default/bug/comments.html.tmpl modified template/en/default/bug/dependency-tree.html.tmpl modified template/en/default/bug/edit.html.tmpl modified template/en/default/bug/create/create-guided.html.tmpl modified template/en/default/list/table.html.tmpl modified template/en/default/pages/fields.html.tmpl modified template/en/default/pages/quicksearch.html.tmpl modified template/en/default/pages/release-notes.html.tmpl modified template/en/default/reports/components.html.tmpl modified template/en/default/reports/duplicates.html.tmpl modified template/en/default/reports/keywords.html.tmpl modified template/en/default/search/boolean-charts.html.tmpl Committed revision 6998.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Whiteboard: [waiting on checkin of blocker before being checked in]
Comment 14•14 years ago
|
||
I backported just the release notes patch to 3.6, so that the trunk release notes and the branch release notes will be identical. Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/3.6/ modified template/en/default/pages/release-notes.html.tmpl Committed revision 7001.
Flags: approval3.6+
Target Milestone: Bugzilla 3.8 → Bugzilla 3.6
You need to log in
before you can comment on or make changes to this bug.
Description
•