XBL content should not generate comment nodes

NEW
Unassigned

Status

()

--
major
17 years ago
5 years ago

People

(Reporter: neil, Unassigned)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

17 years ago
While playing around with XBL I have left comments inside <binding> elements.
These comments appear in the DOM (via the DOM inspector). XUL comments do not.

Updated

17 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → Future
So... Where should I strip comments from?  I can strip them from the XBL
document in general, or I can strip them from inside <content> (Or from inside
other random XBL elements, but that could get harder).  Which should we do?

I'm raising severity to major since this keeps us from documenting XBL bindings
propely...
Assignee: hyatt → bz-vacation
Severity: enhancement → major
Status: ASSIGNED → NEW
OS: Windows 95 → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: Future → mozilla1.7alpha
Ian, Neil's afraid to answer my question without your expert guidance.  Could
you provide some?
Keywords: qawanted
QA Contact: jrgmorrison → ian
You don't want Gecko to strip comments from XBL at all. Doing that would mean
authors could not rely on reliable DOMs. You might want to strip all comments
from XBL files that we ship, though, as part of the build process -- either a
simple XML-based identity transform that drops comment nodes or using #-style
comments and running the XBL files through a preprocessor.

At least that would be my opinion.
The real issue here is that bindings in chrome are very fragile -- adding
comments to <content> makes all sorts of stuff that uses explicit child offsets
break.

This is a problem for authors as much as for chrome, if they use explicit
offsets that way.

Stripping in a preprocessing step is highly suboptimal, since it would make
chrome XBL not behave like other XBL...

We need to either strip comments across the board, or wontfix this bug, and fix
the chrome XBL to not be fragile and finally add some friggin' comments to the
undocumented mess that is chrome XBL.
Well, using #-style comments would clearly mark that these are not XML comments,
which would be pretty clear.
It will also totally confuse the XML-mode in any sane editor and prevent proper
validity-checking from being performed by said editor....

If it's clearly specified that comments are stripped out of XBL, where is the
"could not rely on reliable DOMs" coming in?  I guess for authors that want
their XBL to work with both current and future Mozilla builds?  But those aren't
really using comments anyway, because it's too painful....
> It will also totally confuse the XML-mode in any sane editor

No it won't, it would just be treated as text, which is fine.


> and prevent proper validity-checking from being performed by said editor....

You are unlikely to be validating XBL anyway. Well-formedness checking works
fine with such comments. (Well, assuming you don't actually include tag names in
the comments, but that's easy to avoid.)


> If it's clearly specified that comments are stripped out of XBL, where is the
> "could not rely on reliable DOMs" coming in?  I guess for authors that want
> their XBL to work with both current and future Mozilla builds?  But those 
> aren't really using comments anyway, because it's too painful....

I don't see why the spec should require that UAs have special code for their XBL
content models as opposed to other models. Why should XBL be any different than
any other XML language, like, say, XHTML?
> You are unlikely to be validating XBL anyway

Right.  Instead you're going to be wasting your time building chrome, starting
the browser, and looking at the JS console... That makes so much more sense.  

Avoiding stuff in comments is not a good idea.  The goal here is to make the
barrier to documenting XBL low enough that people will do it.  That means
comments need to Just Work somehow.  Adding comments should not require changes
to any other part of the binding. Everything else is secondary in my opinion. I
don't want to ever again be told that a review comment can't be addressed
because XBL is incapable of usefully and painlessly supporting comments.

The spec is defining how <children> works.  It could clearly say that all
non-comment things in <children> are moved over to be anonymous children of the
bound element.  Nothing to do with the actual XBL content model, which the
author can't ever access anyway (I don't believe there are ways of getting a
hold of the actual XBL document from inside a binding; "document" refers to the
document of the element the binding is attached to).

Another alternative is to add an <xbl:comment> which will be stripped out by the
content sink.  This has the same "avoiding" issue that the preprocessor comments do.

Ccing hyatt (should have done this when I took the bug), since he should
probably be in on this decision and since he may have a proposal for a solution
that you'll like more, Ian.

Comment 9

15 years ago
You guys may not know this, but XBL already does evil processing of anonymous content.  Specifically it 
strips whitespace (even though it should not).  It had to strip whitespace because XUL strips 
whitespace, and we all know that's wrong.

We should make that stripping function just strip comments too, and then find some way to do this 
only for XUL.  Since we need to remain backwards-compatible probably, I'm not sure what a good 
solution is here, since what to do with <content> really depends on the bound element's document 
(XUL vs. HTML) and not on the binding document.

Comment 10

15 years ago
A good solution might be to initially parse everything (comments and whitespace) and then make the 
decision on whether to strip the template in the XBL document based off of the first bound element that 
gets attached.  If it's a XUL document do the strip, and if it isn't XUL, then don't.

This does mean we'll waste time processing comments, but maybe after this initial step, we could add a 
little hint to the <binding> tag for our chrome XBL like xul="true" or something cheesy like that.
(Reporter)

Comment 11

15 years ago
How about stripping whitespace/comments if the XUL namespace is defined?
We shouldn't be doing _any_ stripping of any kind on anything, unless it uses
our XUL MIME type, or is bound to an element in a document that used the XUL
MIME type. I could live with stripping comments and whitespace from XBL (and
XUL) in those cases, though.

However, that will probably not address bz's concern since he wants to be able
to rely on using index-based lookup reliably, and this would not work across
document types.

Personally I'm against all stripping at runtime. (At compile time we should
strip as much as possible, IMHO.) The XML specs actually allow comment
stripping, and whitespace stripping if xml:preserve isn't "preserve", FWIW.
As Neil pointed out on irc, <xbl:comment><!-- --></xbl:comment> would actually
be a simplish way of doing a comment that could be stripped out either at
runtime or compile time, as desired....
Where you should read "strip" in that last comment as meaning "assign special
semantics to, just like <xbl:children />" (which we also "strip" if you think
about it).
<xbl:comment/> feels hacky but I could live with it. It at least makes good
sense in terms of the specs. :-)
(Reporter)

Updated

15 years ago
Blocks: 229043
So do we have agreement?  Should I add support for <xbl:comment> to the XBL
content sink?
Moving bugs that I'm clearly not going to work on in the foreseeable future out
of buglist.
Assignee: bzbarsky → hyatt
Priority: P1 → --
Target Milestone: mozilla1.7alpha → ---
QA Contact: ian → xbl

Updated

9 years ago
Assignee: hyatt → nobody
Dropping qawanted based on comment 18.
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.