Make the Bugzilla Sidebar use valid XUL

RESOLVED FIXED in Bugzilla 2.16

Status

()

P3
critical
RESOLVED FIXED
17 years ago
6 years ago

People

(Reporter: caillon, Assigned: caillon)

Tracking

2.15
Bugzilla 2.16

Details

(URL)

Attachments

(1 attachment)

Filing this as a blocker since we should fix this before the intial release of
the sidebar (2.16), but I have a patch to fix this, and some additional cleanup
on the file.

Before I attach the patch though, has anyone seen the sidebar lately?  Are the
blue link underlines going across the whole sidebar per the design?  Should we
change that?  And anyway, why are we using XUL to begin with?  Can't we just use
HTML/XHTML, but still restrict its usage to Mozilla only?
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → Bugzilla 2.16
What actual problems does the invalid XUL create?
Created attachment 80143 [details] [diff] [review]
Patch v1.0

The 'bare minimum'.  No changes except to make it use valid syntax, and to make
the templating code suck less.	I guess we can do the rest of the items I
mentioned in comment #0 in another bug...
> What actual problems does the invalid XUL create?

The same problems as the invalid HTML bug we had.  Except in this case there's a
bit more.  View the source of the sidebar (CTRL+U).  You will see the second
<script> tag gets treated as JavaScript code, and not a <script> tag because of it.
Keywords: patch, review
scc updated his one recently, I believe - do we want to just copy those changes,
or something?

Dos he even know that we're using his?

Comment 5

17 years ago
Why is this a blocker for 2.16?
I was wondering the same.  Fixing it for 2.16 will be nice, if someone reviews
the patch by then, but I'm certainly not holding the release for it.  I can't
review this because I'm absolutely clueless about XUL. :-)
Severity: blocker → critical
Sorry, my comment #3 was not clear enough.  I filed this as a blocker because
the sidebar won't work half the time without fixing this bug.  If we intend on
releasing the sidebar, let's not release a half-assed one that only works
sometimes if we can help it.

Plus this release seems to be focused on conforming to standards.  Conforming to
XML standards should be the first and foremost standard you should conform to. 
The HTML standard is nice to conform to, but browsers render it ok if it's not.
 Browsers *do not* nor should they ever render properly when it's invalid XML. 
The reason we render the page at all is because there is no XUL doctype to parse
against.

That said, you don't need to know XUL to review this bug.  The bulk of changes
are in template language, with the rest in general XML.

One change is fixing a // comment to be a <!-- comment -->.  Note where it is,
anyone that knows HTML will be able to review that.

The other change is just adding <![CDATA[ ... ]]> blocks around scripts.  It is
needed.  Just have a look at any of the XUL files in the Mozilla source code to
verify this.  Two examples are
http://lxr.mozilla.org/mozilla/source/editor/ui/composer/content/editorPrefsOverlay.xul
http://lxr.mozilla.org/mozilla/source/extensions/cookie/resources/content/cookieNavigatorOverlay.xul#32

I wanted to set this up on landfill but my account is still not activated yet. 
Dave, could you set this up and look at the sidebar, and the source code?  I
would give this r=2x myself since it is "obvious" (to me at least) but as it's
my patch I can't.  Can I at the least get a rubber-stamp if it works for you?  :)
Comment on attachment 80143 [details] [diff] [review]
Patch v1.0

r=bbaetz

I would have preferred critical, not blocker, but we really should do this.
Attachment #80143 - Flags: review+
Comment on attachment 80143 [details] [diff] [review]
Patch v1.0

r=gerv. Port it to the template/en/default/sidebar.xul.tmpl template, and I'll
check it in.

Gerv
Attachment #80143 - Flags: review+
Thanks for the heads up as to the other file, Gerv.  Both are now checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 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.