Remove all presentational markup from Bugzilla for HTML5 compliance

RESOLVED FIXED in Bugzilla 5.0

Status

()

enhancement
RESOLVED FIXED
10 years ago
6 years ago

People

(Reporter: GPHemsley, Assigned: LpSolit)

Tracking

({html5, meta, student-project})

Dependency tree / graph

Details

()

Attachments

(3 obsolete attachments)

Reporter

Description

10 years ago
The link details all of the obsolete markup considered presentational. Bugzilla has a ton of such markup. This bug will get multiple patches to eliminate these atrocities.

I'll try to keep them short, sweet, and to the point.
Reporter

Comment 1

10 years ago
Posted patch Starting point (obsolete) — Splinter Review
All patches in this bug should be applied after attachment 427539 [details] [diff] [review].

This starting point patch adds some basic default styling to take care of some of the more obvious and repetitive presentational markup. (I anticipate having to modify this some more as development goes on.)

This patch also brings the header (and footer, which needed no changes) up to code, which is always a good starting point.
Reporter

Comment 2

10 years ago
Posted patch Remove <th align="left"> (obsolete) — Splinter Review
This removes all instances of <th align="left">, since that should be the default alignment anyway.
  Okay, each of these should go on a separate bug, actually.

  Also, you're going to want to request review for them, or they'll just sit here.

(In reply to comment #1)
> All patches in this bug should be applied after attachment 427539 [details] [diff] [review].

  Actually, I think we're going to do that last. If this all works, browsers will render the html5 identically as they do the html4.
Reporter

Updated

10 years ago
Depends on: 547311
Reporter

Comment 4

10 years ago
(In reply to comment #3)
>   Okay, each of these should go on a separate bug, actually.
> 
>   Also, you're going to want to request review for them, or they'll just sit
> here.

Alright, I'll break them up by attribute and then request reviews when all the patches for that attribute are ready.

> (In reply to comment #1)
> > All patches in this bug should be applied after attachment 427539 [details] [diff] [review] [details].
> 
>   Actually, I think we're going to do that last. If this all works, browsers
> will render the html5 identically as they do the html4.

Per the logic presented in IRC, that sounds fair.
Keywords: meta
Reporter

Comment 5

10 years ago
Comment on attachment 427734 [details] [diff] [review]
Remove <th align="left">

This patch is now attached to bug 547311 as attachment 427833 [details] [diff] [review].
Attachment #427734 - Attachment is obsolete: true
Reporter

Updated

10 years ago
Attachment #427728 - Flags: review?(mkanat)
Comment on attachment 427728 [details] [diff] [review]
Starting point

>=== modified file 'skins/contrib/Dusk/global.css'
>+/* Bad web designers. Bad. */
>+table.for-layout {
>+    border-spacing: 0em;
>+    border-width: 0px;
>+}

  Hmm. Why not just make that the default table CSS, and then fix any tables that it messes up?

  Also, the comment is amusing, but it will be in Bugzilla forever, so it should probably go. :-)

>+form tr th:first-child {

  first-child doesn't work in older browsers, for sure. I'm pretty sure it doesn't even work in IE7.
Attachment #427728 - Flags: review?(mkanat) → review-
Reporter

Comment 7

10 years ago
(In reply to comment #6)
> (From update of attachment 427728 [details] [diff] [review])
> >=== modified file 'skins/contrib/Dusk/global.css'
> >+/* Bad web designers. Bad. */
> >+table.for-layout {
> >+    border-spacing: 0em;
> >+    border-width: 0px;
> >+}
> 
>   Hmm. Why not just make that the default table CSS, and then fix any tables
> that it messes up?

Because tables aren't supposed to be for layout. Tables actually used for tabular data (i.e. used properly) should have their borders and padding show as normal.

>   Also, the comment is amusing, but it will be in Bugzilla forever, so it
> should probably go. :-)

It shouldn't be in there forever, because the tables-for-layout shouldn't be in there forever. ;)

> >+form tr th:first-child {
> 
>   first-child doesn't work in older browsers, for sure. I'm pretty sure it
> doesn't even work in IE7.

True. However, my thinking was that the browsers that don't support it would just get left-aligned header cells, which is certainly not the end of the world.
(In reply to comment #7)
> Because tables aren't supposed to be for layout. Tables actually used for
> tabular data (i.e. used properly) should have their borders and padding show as
> normal.

  Even so, when I have a real table, I want to control the layout totally, I don't want to have the browser's default formatting on it.

  Also, in reality, there are some types of layout that are difficult-to-impossible to do without tables. (I am *really* good with CSS, and this is true even for me.)

> It shouldn't be in there forever, because the tables-for-layout shouldn't be in
> there forever. ;)

  They may be, though. In any case, let's remove the comment.

> True. However, my thinking was that the browsers that don't support it would
> just get left-aligned header cells, which is certainly not the end of the
> world.

  Well, yes, but in this case, "browsers that don't support it" represent 50% of the Internet. Also, why only the first-child, for this?
Reporter

Comment 9

10 years ago
(In reply to comment #8)
> (In reply to comment #7)
> > Because tables aren't supposed to be for layout. Tables actually used for
> > tabular data (i.e. used properly) should have their borders and padding show as
> > normal.
> 
>   Even so, when I have a real table, I want to control the layout totally, I
> don't want to have the browser's default formatting on it.

Alright, but I can tell you that that's not the case with current real tables in Bugzilla.

>   Also, in reality, there are some types of layout that are
> difficult-to-impossible to do without tables. (I am *really* good with CSS, and
> this is true even for me.)

Yeah, I really wish they would do something about that....

> > It shouldn't be in there forever, because the tables-for-layout shouldn't be in
> > there forever. ;)
> 
>   They may be, though. In any case, let's remove the comment.

Should I replace it with something else, or just remove it entirely?

> > True. However, my thinking was that the browsers that don't support it would
> > just get left-aligned header cells, which is certainly not the end of the
> > world.
> 
>   Well, yes, but in this case, "browsers that don't support it" represent 50%
> of the Internet. Also, why only the first-child, for this?

Hmm... well, it's for the cases where you have a form and a two-column table, with the left column (<th>) for the title of the input and the right column (<td>) for the input itself.
(In reply to comment #9)
> >   Even so, when I have a real table, I want to control the layout totally, I
> > don't want to have the browser's default formatting on it.
> 
> Alright, but I can tell you that that's not the case with current real tables
> in Bugzilla.

  Okay, so maybe we should have a patch to fix that first. I really want tables to default to having 0 cell spacing/padding.

> Should I replace it with something else, or just remove it entirely?

  Hmm, let's just remove it entirely for now.

> > of the Internet. Also, why only the first-child, for this?
> 
> Hmm... well, it's for the cases where you have a form and a two-column table,
> with the left column (<th>) for the title of the input and the right column
> (<td>) for the input itself.

  Oh, do we have a fair number of those?
Reporter

Comment 11

10 years ago
(In reply to comment #10)
> (In reply to comment #9)
> > >   Even so, when I have a real table, I want to control the layout totally, I
> > > don't want to have the browser's default formatting on it.
> > 
> > Alright, but I can tell you that that's not the case with current real tables
> > in Bugzilla.
> 
>   Okay, so maybe we should have a patch to fix that first. I really want tables
> to default to having 0 cell spacing/padding.

OK, if that's what you want, it's not a problem to change it to all tables.
 
> > > of the Internet. Also, why only the first-child, for this?
> > 
> > Hmm... well, it's for the cases where you have a form and a two-column table,
> > with the left column (<th>) for the title of the input and the right column
> > (<td>) for the input itself.
> 
>   Oh, do we have a fair number of those?

Approximately 75.
Reporter

Comment 12

10 years ago
Posted patch Starting point, v2 (obsolete) — Splinter Review
This should address the issues with the previous patch.
Attachment #427728 - Attachment is obsolete: true
Attachment #427907 - Flags: review?(mkanat)
Reporter

Updated

10 years ago
Keywords: student-project
Comment on attachment 427907 [details] [diff] [review]
Starting point, v2

  This patch by itself can't be accepted, because it adjusts the CSS of global elements without fixing the rest of Bugzilla.

>+table th, table td {
>+    padding: 0em;
>+}

  Also, that should just be "th" and "td"--they only appear in tables, and setting them as just a simple element makes it easier to override the style.

>+form tr th.form-label {
>+	text-align: right;
>+}

  Similarly, that should just be form th.form-label, although I don't think that form-label is a good class name, because there are lots of form labels that align different ways.
Attachment #427907 - Flags: review?(mkanat) → review-

Updated

9 years ago
Target Milestone: Bugzilla 4.0 → Bugzilla 5.0
Assignee

Comment 14

7 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

Updated

6 years ago
Depends on: 840407
Assignee

Updated

6 years ago
Depends on: 920681
Assignee

Updated

6 years ago
Depends on: 322402
Assignee

Updated

6 years ago
Depends on: bz-css
Assignee

Updated

6 years ago
Depends on: 952793
Assignee

Updated

6 years ago
Depends on: 952796
Assignee

Updated

6 years ago
Attachment #427907 - Attachment is obsolete: true
Assignee

Comment 15

6 years ago
All done. All templates are (should be) valid HTML5 pages now.
Assignee: ui → LpSolit
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.