Closed Bug 107120 Opened 22 years ago Closed 22 years ago

templates/default/global/header has invalid HTML

Categories

(Bugzilla :: User Interface, defect, P1)

2.15
defect

Tracking

()

RESOLVED FIXED
Bugzilla 2.16

People

(Reporter: justdave, Assigned: justdave)

References

Details

Attachments

(2 files, 1 obsolete file)

Warning (1/1): <!DOCTYPE> is missing.
Error (9/3): In the tag <BODY> white space is missing as separator after the
attribute "ALINK". 
Warning (19/9): In the tag <TD> the attribute "WIDTH" should only contain
absolute pixel values.
Error (28/9): In tag <TD> the value "center" is not valid for attribute "VALIGN". 
Error (29/9): In tag <TD> the value "center" is not valid for attribute "VALIGN".
Blocks: 47251
Severity: normal → blocker
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → Bugzilla 2.16
Assignee: myk → justdave
Status: ASSIGNED → NEW
er, I meant to assign to me first before I accepted that...  really I did... :-)
Attached patch patch v1Splinter Review
the space thing after headerhtml, is that a bug in Template Toolkit, or is it
supposed to ignore whitespace between two variable substitutions?
Comment on attachment 55385 [details] [diff] [review]
patch v1

The template toolkit appears to honor all whitespace outside to the [% %] tags.
If you don't want the blanks line at the begining, you'll have to put the doc
type declaration before the DEFAULT block, eg:

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
[% DEFAULT
  title = ""
  h1 = title
  h2 = ""
  extra = ""
  jscript = ""
  style = ""
  message = ""
%]
<html>

Or on the same line as the %]

Not that it matters terribly much, so r= jake
Attachment #55385 - Flags: review+
Comment on attachment 55385 [details] [diff] [review]
patch v1

I think that needing the [% " " %] is either a bug or a misfeature in the template toolkit, but...

r=bbaetz
Attachment #55385 - Flags: review+
If <body [% Param('bodyhtml') %] [% extra %]>
is broken, just do:
+  <body [% Param('bodyhtml') %] 
         [% extra %]>

That should surely work.

But is anyone going to check this in?

Gerv

Keywords: patch, review
Checked in.

/cvsroot/mozilla/webtools/bugzilla/template/default/global/header,v  <--  header
new revision: 1.2; previous revision: 1.1
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
HTML 4.01 Transitional demands that the <style> tag have a "type"
attribute.

http://validator.w3.org/check?uri=http%3A%2F%2Flandfill.tequilarista.org%2Fbbaetz%2Fquery.cgi&charset=%28detect+automatically%29&doctype=Inline&ss=
Furthermore, HTML 4.01 Transitional demands a character set encoding
be specified for the page.  No character encoding (i.e., "utf-8" or 
"iso-8559-1") is set in the header using a <meta> tag.  Since not all
web servers emit a charset encoding in their HTTP headers (for example,
bugzilla.mozilla.org does not, but landfill.tequilarista.org does),
this should be set for all pages.

See the very first warning from this example URL for details.

http://validator.w3.org/check?uri=http%3A%2F%2Fbugzilla.mozilla.org%2Findex.html&charset=%28detect+automatically%29&doctype=%28detect+automatically%29&ss=

Patch to follow soon.  "iso-8859-1" may be replaced with "utf-8" if
desired in the patch.  Jake mentioned on IRC that there was a bug
about choosing a character set encoding for Bugzilla web pages, but
I could not find one using search terms like charset, character set,
encoding, meta, utf-8 or iso-8859-1.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch Additional HTML fixes (obsolete) — Splinter Review
- Adds "type" attribute to <style> tag.
- Adds <meta> tag to specify character set encoding (i.e., utf-8 or
  iso-8859-1)
See bug 38856... it ended up being a docs item.
- Remove <meta> tag per bug 38856
Attachment #58368 - Attachment is obsolete: true
Comment on attachment 58369 [details] [diff] [review]
Additional HTML fixes v2

r=caillon.

I don't think this needs second review.  Clear cut case of being standards
compliant or not.  Very simple patch.
Attachment #58369 - Flags: review+
Fix checked in
Status: REOPENED → RESOLVED
Closed: 22 years ago22 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.