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?
17 years ago
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...
17 years ago
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?
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.
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
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
You need to log in before you can comment on or make changes to this bug.