Closed
Bug 547311
Opened 14 years ago
Closed 11 years ago
Remove the "align" attribute from <th>
Categories
(Bugzilla :: User Interface, enhancement)
Bugzilla
User Interface
Tracking
()
RESOLVED
FIXED
Bugzilla 5.0
People
(Reporter: GPHemsley, Assigned: chtrom)
References
Details
(Keywords: html5, student-project)
Attachments
(2 files, 6 obsolete files)
27.41 KB,
patch
|
LpSolit
:
review+
|
Details | Diff | Splinter Review |
7.41 KB,
patch
|
mail
:
review+
|
Details | Diff | Splinter Review |
Remove the align attribute from Bugzilla templates towards the goal of removing presentational markup for HTML5 compliance. (This attachment is identical to attachment 427734 [details] [diff] [review] from bug 547171 comment 2.)
Comment 1•14 years ago
|
||
<th> defaults to centered alignment, not left, I'm pretty sure.
Reporter | ||
Comment 2•14 years ago
|
||
(In reply to comment #1) > <th> defaults to centered alignment, not left, I'm pretty sure. Oh, darn it, you're right. Alright, back to the drawing board.
Reporter | ||
Updated•14 years ago
|
Attachment #427833 -
Attachment is obsolete: true
Attachment #427833 -
Flags: review-
Comment 3•14 years ago
|
||
you might want to look at YUI's reset css. It has a lot of stuff to help make things uniform across browsers
Reporter | ||
Comment 4•14 years ago
|
||
This changes all instances of <th align="right"> to <th class="form-label">. This class should be used whenever there is a form in a two-column layout with a <th> column on the left (the labeler) and a <td> column on the right (the labelee).
Assignee: ui → gphemsley
Status: NEW → ASSIGNED
Reporter | ||
Comment 5•14 years ago
|
||
This is a companion to attachment 427909 [details] [diff] [review]. It replaces certain instances of <th align="left"> with <th class="form-label">.
Reporter | ||
Updated•14 years ago
|
Keywords: student-project
Reporter | ||
Updated•14 years ago
|
Assignee: gphemsley → ui
Status: ASSIGNED → NEW
Updated•14 years ago
|
Target Milestone: Bugzilla 4.0 → Bugzilla 5.0
Comment 6•12 years ago
|
||
Hum, nobody noticed these patches because they have no requests for review.
Comment 7•12 years ago
|
||
We are going to branch for Bugzilla 4.4 next week and this bug is too invasive or too risky to be accepted for 4.4 at this point. The target milestone is set to 5.0 which is our next major release. I ask the assignee to reassign the bug to the default assignee if you don't plan to work on this bug in the near future, to make it clearer which bugs should be fixed by someone else on time for 5.0.
Target Milestone: Bugzilla 4.4 → Bugzilla 5.0
Assignee | ||
Comment 8•12 years ago
|
||
If the patches provided are not satisfactory, I would like to work on this bug.
Assignee | ||
Comment 9•12 years ago
|
||
I cannot find the class (form-label) that was referenced in the previous patches. It looks to me like all that is needed is classes that apply the "text-align:left;" style instead of align="left" and "text-align:right;" instead of align="right". What naming convention should I use for these classes?
Comment 10•12 years ago
|
||
(In reply to Christopher from comment #9) > I cannot find the class (form-label) that was referenced in the previous > patches. It looks to me like all that is needed is classes that apply the > "text-align:left;" style instead of align="left" and "text-align:right;" > instead of align="right". What naming convention should I use for these > classes? Simply define .form-label into global.css which sets align="right". I don't think we need a class for align="left". This last one is used in showdependencygraph.cgi only, and we could as well use .form-label (i.e. right-aligned) there too.
Assignee: ui → chtrom
Assignee | ||
Comment 11•12 years ago
|
||
This patch changes every instance of align="right" to class="form-label".
Attachment #682907 -
Flags: review?(LpSolit)
Comment 12•12 years ago
|
||
Comment on attachment 682907 [details] [diff] [review] Replace align="right" with class="form-label" You forgot to add this class in global.css. This should be part of this patch.
Attachment #682907 -
Flags: review?(LpSolit) → review-
Assignee | ||
Comment 13•12 years ago
|
||
This patch removes align="right", align=right, and align="left" from every <th> tag in the template directory and adds "form th { text-align: right; }" to global.css.
Attachment #682907 -
Attachment is obsolete: true
Attachment #682939 -
Flags: review?(LpSolit)
Updated•12 years ago
|
Attachment #427909 -
Attachment is obsolete: true
Updated•12 years ago
|
Attachment #427910 -
Attachment is obsolete: true
Comment 14•12 years ago
|
||
Comment on attachment 682939 [details] [diff] [review] Remove align=left and align=right from every <th> Looks good. Thanks! :) r=LpSolit
Attachment #682939 -
Flags: review?(LpSolit) → review+
Comment 15•12 years ago
|
||
Subsequent bugs must be filed to remove remaining align="..." attributes from other HTML elements. This bug is specific to <th>. Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/ modified skins/standard/global.css modified template/en/default/account/auth/login.html.tmpl modified template/en/default/account/email/confirm-new.html.tmpl modified template/en/default/account/email/confirm.html.tmpl modified template/en/default/account/password/set-forgotten-password.html.tmpl modified template/en/default/account/prefs/account.html.tmpl modified template/en/default/admin/classifications/edit-common.html.tmpl modified template/en/default/admin/classifications/edit.html.tmpl modified template/en/default/admin/classifications/reclassify.html.tmpl modified template/en/default/admin/classifications/select.html.tmpl modified template/en/default/admin/custom_fields/edit-common.html.tmpl modified template/en/default/admin/fieldvalues/create.html.tmpl modified template/en/default/admin/fieldvalues/edit.html.tmpl modified template/en/default/admin/keywords/create.html.tmpl modified template/en/default/admin/keywords/edit.html.tmpl modified template/en/default/admin/milestones/create.html.tmpl modified template/en/default/admin/products/confirm-delete.html.tmpl modified template/en/default/admin/products/create.html.tmpl modified template/en/default/admin/products/edit-common.html.tmpl modified template/en/default/admin/products/edit.html.tmpl modified template/en/default/admin/versions/create.html.tmpl modified template/en/default/admin/workflow/comment.html.tmpl modified template/en/default/admin/workflow/edit.html.tmpl modified template/en/default/bug/dependency-graph.html.tmpl modified template/en/default/global/choose-classification.html.tmpl modified template/en/default/global/choose-product.html.tmpl modified template/en/default/reports/keywords.html.tmpl modified template/en/default/whine/mail.html.tmpl modified template/en/default/whine/schedule.html.tmpl Committed revision 8503.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Summary: Remove @align from Bugzilla templates → Remove the "align" attribute from <th>
Assignee | ||
Comment 16•11 years ago
|
||
Several instances of the align attribute on <th> elements were missed in the previous patch. This patch removes those which were missed and adds class "left" to fix those which require left text alignment.
Attachment #754600 -
Flags: review?(LpSolit)
Assignee | ||
Updated•11 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 17•11 years ago
|
||
Comment on attachment 754600 [details] [diff] [review] Remove remaining align attributes from <th>'s >=== modified file 'skins/standard/admin.css' >+th.left { >+ text-align: left; >+} >=== modified file 'skins/standard/attachment.css' >+th.left { >+ text-align: left; >+} Why not putting th.left in global.css directly? That's where we put "form th" in the previous patch.
Assignee | ||
Comment 18•11 years ago
|
||
(In reply to Frédéric Buclin from comment #17) > > Why not putting th.left in global.css directly? That's where we put "form > th" in the previous patch. Because putting it in these two files covered all of the known cases. If it were put in global.css, many pages that didn't need it would still load it.
Comment 19•11 years ago
|
||
(In reply to Christopher Trom from comment #18) > Because putting it in these two files covered all of the known cases. If it > were put in global.css, many pages that didn't need it would still load it. But we wouldn't have to add this CSS rule elsewhere the next time we add a new table in a template which doesn't load these 2 CSS files. :) But that's not that important.
Assignee | ||
Comment 20•11 years ago
|
||
(In reply to Frédéric Buclin from comment #19) > But we wouldn't have to add this CSS rule elsewhere the next time we add a > new table in a template which doesn't load these 2 CSS files. :) But that's > not that important. Do you want me to move the rule into global.css?
Updated•11 years ago
|
Attachment #754600 -
Flags: review?(LpSolit) → review?(sgreen)
Comment 21•11 years ago
|
||
(In reply to Christopher Trom from comment #20) > Do you want me to move the rule into global.css? Yes. I think the rule should only be defined once in CSS. If it is used in many places, then global.css is the correct place.
Updated•11 years ago
|
Attachment #754600 -
Flags: review?(sgreen) → review-
Assignee | ||
Comment 22•11 years ago
|
||
Moved CSS rule into global.css.
Attachment #754600 -
Attachment is obsolete: true
Attachment #791591 -
Flags: review?(sgreen)
Comment 23•11 years ago
|
||
Comment on attachment 791591 [details] [diff] [review] patch.diff >=== modified file 'template/en/default/global/choose-product.html.tmpl' >--- template/en/default/global/choose-product.html.tmpl 2013-08-15 23:08:20 +0000 >+++ template/en/default/global/choose-product.html.tmpl 2013-08-17 01:25:53 +0000 >@@ -37,7 +37,7 @@ > [% FOREACH c = classifications %] > [% IF c.object %] > <tr> >- <th colspan="2" align="left">[% c.object.name FILTER html %]: >+ <th colspan="2">[% c.object.name FILTER html %]: > [%+ c.object.description FILTER html_light %]</th> > </tr> > [% END %] > This change needs class="left" otherwise the select new bug product page looks not so nice.
Attachment #791591 -
Flags: review?(sgreen) → review-
Assignee | ||
Comment 24•11 years ago
|
||
Oops, sorry I missed that.
Attachment #791591 -
Attachment is obsolete: true
Attachment #794929 -
Flags: review?(sgreen)
Updated•11 years ago
|
Attachment #794929 -
Flags: review?(sgreen) → review+
Updated•11 years ago
|
Flags: approval?
Updated•11 years ago
|
Flags: approval? → approval+
Comment 25•11 years ago
|
||
Thanks for the patch. Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/ modified skins/standard/global.css modified template/en/default/admin/table.html.tmpl modified template/en/default/admin/classifications/del.html.tmpl modified template/en/default/admin/components/confirm-delete.html.tmpl modified template/en/default/admin/custom_fields/confirm-delete.html.tmpl modified template/en/default/admin/fieldvalues/confirm-delete.html.tmpl modified template/en/default/admin/fieldvalues/edit.html.tmpl modified template/en/default/admin/milestones/confirm-delete.html.tmpl modified template/en/default/admin/products/confirm-delete.html.tmpl modified template/en/default/admin/versions/confirm-delete.html.tmpl modified template/en/default/attachment/confirm-delete.html.tmpl modified template/en/default/attachment/diff-file.html.tmpl modified template/en/default/attachment/list.html.tmpl modified template/en/default/global/choose-product.html.tmpl Committed revision 8720.
Status: REOPENED → RESOLVED
Closed: 12 years ago → 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•