547171 - Remove all presentational markup from Bugzilla for HTML5 compliance

Status: RESOLVED FIXED

(Bugzilla :: User Interface, enhancement)

Description

[Gordon P. Hemsley [:GPHemsley]]

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.

Comment 1

[Gordon P. Hemsley [:GPHemsley]]

Attachment: Starting point

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.

Comment 2

[Gordon P. Hemsley [:GPHemsley]]

Attachment: Remove <th align="left">

This removes all instances of <th align="left">, since that should be the default alignment anyway.

Comment 3

[Max Kanat-Alexander]

  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.

Comment 4

[Gordon P. Hemsley [:GPHemsley]]

(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.

Comment 5

[Gordon P. Hemsley [:GPHemsley]]

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].

Comment 6

[Max Kanat-Alexander]

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.

Comment 7

[Gordon P. Hemsley [:GPHemsley]]

(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.

Comment 8

[Max Kanat-Alexander]

(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?

Comment 9

[Gordon P. Hemsley [:GPHemsley]]

(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.

Comment 10

[Max Kanat-Alexander]

(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?

Comment 11

[Gordon P. Hemsley [:GPHemsley]]

(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.

Comment 12

[Gordon P. Hemsley [:GPHemsley]]

Attachment: Starting point, v2

This should address the issues with the previous patch.

Comment 13

[Max Kanat-Alexander]

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.

Comment 14

[Frédéric Buclin]

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.

Comment 15

[Frédéric Buclin]

All done. All templates are (should be) valid HTML5 pages now.