- From: Jonas Sicking <jonas@sicking.cc>
- Date: Mon, 26 Mar 2007 19:56:48 -0300
- To: Ian Hickson <ian@hixie.ch>
- Cc: public-appformats@w3.org
These things take a long time to reply to, sorry about that. Especially when writing this from the beach of Buenos Aires :) Ian requested that I split my comments into ones that are important to me as an implementor, and ones that are not. I also made a category for ones that are more comments that I think benefit users for one reason or other, and a category for clarification requests (i.e. comments not requesting functional changes to the spec, but editorial ones) == Requests for clarification (or at least I think they are) == >> 1.4.2) >> Why are selectors case insensitive? > > That's a question for the CSS working group. Since you're only referring to prefixes, please state so in the text. However I disagree that the spec actually says so. The spec for the @namespace rule does indeed say that it is case insensitive. However the selectors spec doesn't really say anything one way or the other. >> 3.6) >> What functions should be called if the xblEnteredDocument function >> implementation moves the bound element to a different document? > > An xblLeftDocument and an xblEnteredDocument. These methods are called > when everything else is stable. So nothing would happen until the > first xblEnteredDocument function returned, then, after everything has > settled down, the UA would fire an xblLeftDocument, and if nothing > changed in that time, an xblEnteredDocument. Does that make sense? I did realize that since the node is now in a new document it'll be a completely different binding that gets attached to it. It may still be a good idea to state the xblLeftDocument can be called even when the bound element is positioned inside a document in this case. >> 4.1) >> What is "xml:base data"? Same thing as the xml:base attribute? > > It's the data resulting from the use of xml:base. So in your > implementation, probably just the attribute, yes. I'm not following what the difference is. In any case the term "xml:base data" is not defined anywhere in the spec. I suggest either using the term "xml:base attribute" or defining the term "xbl:base data" properly in the spec. >> 4.4.1) >> "Matching of the elements to the selector is done without taking into >> account the shadow tree in which the content element itself is found". >> Are other bindings on the bound element taken into account? In general >> this part feels too vaguely defined, what if there are implicitly or >> explicitly derived bindings, are they taken into account? > > I don't understand the question. Could you give an example that you > believe is ambiguous? Since it says "without taking into account the shadow tree in which the content element itself is found" it sounds to me like some other shadow tree is taken into account. My question is which ones. I take it no bindings for the current explicit parent needs to be taken into account, i.e. you can act as if the child is an explicit child of the element that you're currently creating the shadow tree for. Let me see if I can come up with some better wording that makes this more clear. Suggestions of course welcome :) >> Same thing with <inherits> inside <content> and <content> inside >> <inherits>. In other words, what does the following three templates >> do? <inherited><content/></inherited> > > The <inherited> element contents are used when there is no base > binding. If a node is assigned to a <content> element that doesn't end > up in the final flattened tree, well, it doesn't end up in the final > flattened tree. So in other words the explicit children are distributed to the "DOM Core tree", independent on if parts of that tree is then hidden in the flattened tree. This would be good to state explicitly. >> 4.5) >> The first two paragraphs make it sound like the DOM is mutated, which >> should not be the case. > > I'm not sure how to fix this. Do you have any suggested text? Note > that there is an additional paragraph there now that should makes > things a bit clearer. Add a sentence somewhere in there that says something to the extent of: "These steps described here do not change the actual bound document or the shadow trees. They are simply describing the process that would generate a tree that is like the final flattened tree. The actual DOM nodes in the bound document and in the shadow trees are left intact" >> 4.7.1) >> It is unclear if when allow-selectors-through is true and an explicit >> child of a bound element is inserted in a binding using <content> if the >> parents of the <content> take part in parent chain as far as selector >> matching goes. > > I don't understand what interpretation could lead you to assume that > it does. Could you elaborate on why you read it that way? There's nothing in the text to directly suggest it. However I think it's easy to get the impression that the > operator works on the flattened tree since many other things operate on the flattened tree, and the > operator does partially too. So I would suggest that it was explicitly stated that that is not the case. >> 4.10) >> "Sheets are always walked from the innermost shadow scope to the >> outermost shadow scope". How does this interact with sheets having >> different CSS origin levels, i.e. some being UA sheets and some >> being author sheets? > > CSS already defines that, no? My question is, does the above paragraph take precedence over the CSS cascade or not? I.e. do you first use all the origin levels at the innermost scope and then all at the second innermost scope? Or do you within each origin level first use the innermost scope and then the second innermost? I guess it makes more sense to within each origin level first use the innermost scope and then the second innermost. If this is already what it defines, this would be good to clarify maybe? >> 6.2) >> "When a handler element is invoked by the object passed to the >> addEventListenerNS() method, the user agent must check that the >> event in question was forwarded to a handlers element from a bound >> element, and that that handlers element is the parent node of the >> handler element." When is that not the case? Especially the parent >> part since I'd assume that removing a handler from the handlers >> element would call removeEventHandlerNS. > > I'm not sure, but I imagine XML Events or other such wacky mechanisms > might do it. But XML Events are not handler elements (Well, they can be I think, but not in the sense that is discussed here, i.e. ones in the xbl namespace), so the above can't apply to them as they're a separate spec. >> Does not setting the phase attribute cause the handler to be >> potentially triggered three times for the same even? I.e. during >> bubble, capture and default phases? > > It can never trigger both during bubble and capture, since the event > is only ever registered for one or the other. It can never trigger > during both bubble and target or capture and target, since the target > element is never given the event in capture or bubble phase. Where does it specify this? Note that even in DOM Events spec you can be notified both during bubbling and capturing if you call addEventListenerNS twice. Additionally, XBL events doesn't map directly to DOM Events due to the default action phase. I guess the real question is, what phase should you be registered for if there is no phase attribute set. >> I don't understand the example for hasBinding at all, it seems to be >> missing context. > > Not sure what makes you think that...? Basically I don't understand the example at all and I can only vaguely guess what it means. But it's vague enough that it doesn't really help me at all. I'll write a new proposal for section 6.8 that should make things clearer but not change what I think the spec is trying to say now. == Comments foremost due to what I think is best for users (due to logical behavior, smaller spec, or otherwise) == >> 1.4.3) >> Why is tab not considered whitespace? > > They are. Sure, now they are ;) >> 3.2.1) >> Why are xbl-PIs inserted in the DOM in error? That runs contrary to most >> things where the current state of the DOM is what matters, not how that >> state was reached. Scripts are so far the only exception to this. Same >> thing applies to dynamic changes to the PI. > > Because otherwise interfaces implemented by elements in the document > could change dynamically. There is already a separate API for doing > this in dynamic environments (loadBindingDocument()). Interfaces implemented by elements can still change dynamically since elements will change which selectors they match and can therefor match new element-attributes or get applied different CSS rules. If you want to break the model of allowing dynamic modifications to the DOM I think you should have a reason for it other than "there are other ways of doing the same thing", as is the case for <script> elements and xmlns attributes. And come to think of it, .loadBindingDocument is not a full replacement since it is syncronous, which in general is a bad idea for authors to use. So if an author want to apply a set of bindings asynconously they could do it by adding a PI. Hey, AJAX is the buzzword of the day right ;) >> 4.3.1) >> Why can xbl:text not appear by itself as a shorthand for xbl:text=xbl:text > > What's the use case for that? Why not just use <content>? This is not so much a request for a feature, but a request for a simplification to both the spec and the implementation. Since xbl:text can appear on either side the implementaion will have to deal with both parts of xbl:text anyway. I would imagine the implementation will have a generic 'reader' part that reacts to whatever is on the right hand side of '=', and a generic 'writer' part that writes to whatever is on the left hand side of the '='. This would work fine even when xbl:text appears by itself. As the spec is written now the parser will specifically have to look for xbl:text and disallow that, even though it's likely that the rest of the implementation would deal fine with it. Note that as the spec is currently written xbl:attr="xbl:text=xbl:text" is allowed, ensuring that the rest of the could would have to be able to deal with this. So it's literally just the attribute parser that would have to make an exception for xbl:text. >> Why are not all explicit text descendants (rather than just children) be >> taken into account when xbl:text appears on the right hand side of the '='. > > Because doing that could result in broken renderings (e.g. losing text > direction information if one of the children is an <html:bdo> > element) in cases the author did not consider. If the author didn't consider <html:bdo> elements then with the current behaviour he'd miss out on parts of the text which seems equally bad. The main argument I can see against copying descendants is actually that it may be harder on implementations to do in a performant way, though for mozilla that shouldn't be a problem. >> does xbl:template > my:B match the B element in the following binding >> <binding><template><my:B/></template></binding>. If yes, that doesn't >> match what is stated. If no, why not? > > It does not, because the ">" combinator is defined to consider the > bound element to be the parent of the shadow tree, not the <template> > element. This allows you to make stylistic choices based on the > attributes and other facets of the bound element (if you set > allow-selectors-through, anyway). I see the usefulness when allow-selectors-through is set, but when it is not set it makes things inconvenient for the author since there is no way to match things that are in the root of the shadow tree. As far as selector matching goes it will look like all elements in the root of the shadow tree are disconnected orphans. >> 4.7.3 >> The dual meaning of the :bound-element pseudo class seems like a bad >> idea from a usability point of view. It also makes it impossible to >> match a bound element inside a shadow tree. Why not split it up into two >> selectors? >> What is the usecase for matching all xbl-bound elements in general? > > It isn't clear that matching a bound element in general is a useful > thing. That this is possible at all is mostly the result of defining > what should happen in the absense of a bound element, rather than the > result of an actual use case. I suggest we remove that feature since it seems just to add unnecessary bloat to implementations. Additionally, it seems harder for authors to use the selector since something like :bound-element{border: red} would not only add a border to the bound element that the current binding is attached to, but also add borders to all bound elements in the bindings shadow tree. >> 5.4) >> Properties from one external binding object needs to be forwarded to >> [external] deriving binding objects. Otherwise using the .baseBinding >> property is fairly useless in that it only contains the properties of >> the immediately inherited binding. > > Not sure I understand this either. Say that you have three bindings, A -> B -> C, with all inheritance being explicit. If the implementation in A overrides a function, setColor, where the implementation needs to call the overridden function you would do this using something like setColor: function(col) { ... this.baseBinding.setColor(col); ... } However this would with the current spec only work if setColor was implemented directly on B. If the function were inherited from C then the above would have to be this.baseBinding.baseBinding.setColor. This is not a good idea since it makes inheritance of implementation less useful and is not how programming languages are usually designed. In order to fix this we should say that forwarding is done from one external object to the next in the inheritance chain. And then just say that the bound element just forwards to the first implementation object. >> 6.9) >> Why the random number used for eventPhase during default-action phase? >> Couldn't a more rememberable one be used? > > It's not a random number (it's the string "xblD" in ASCII read as a > big-endian 32bit integer) but it _is_ an arbitrary number, in that it > shouldn't really matter what its value is -- people aren't expected to > ever look at it. The only way for code to execute when it is in the > default phase is for a <handler> to have phase=default-action, so code > always knows whether phase has this value or not. > > The reason for the crazy number was to avoid clashes with anything > else. Waving a crazy sign in front of someone's eyes saying "don't pay attention to this sign" is bound to get them to focus all their energy on this sign, or at least get them annoyed. I would much prefer to use a more sane number. Say 0x100 or 0x101. If for no other reason simply having crazy numbers like that in specs is bound to confuse people, and I suspect there will be places where you call a common function from different handlers and pass the phase as an argument. >> Wouldn't it make more sense for the default phase to go from the >> document node to the targeted node? This since the final action will >> be on the innermost node, i.e. the targeted node. Additionally, this >> allows the binding to implement a default behavior for the bound >> element without having to worry about what inner elements it is >> using and their default behavior. > > Well, you want it to go inside-out because that's how default > activation behaviours work in DOM3 Events (though this may not yet > have been actually written down anywhere). But yeah, I'm still looking > at this. Implementation experience will be helpful here. I think the most useful experience here will be user experience rather than implementation experience. I can definitely see arguments for going both ways, but generally outside in feels more useful for xbl. >> 6.10) >> Does the :focus handling also apply to :active? I.e. can there be a >> chain of :active elements? > > If you can get the CSS working group to agree on what :active means > _outside_ of XBL, let me know and I'll update the XBL2 spec to > elaborate on what it should do when XBL is involved! Given how browsers today treat it I think it makes sense to say that there is a chain of :active elements too, and possibly revise the other way once the CSS group makes up their mind. >> 2.5) >> Why [doesn't] removing the locked attribute reset the list of included >> children? > > Why should it? Because the DOM generally behaves according to its current state, independent of how it got to that state. To put it the other way, why would the user remove the locked attribute if he didn't want the element to behave unlocked? Additionally, this doesn't match the steps in 4.4.1 >> And does the includes attribute have any effect when locked is true? > > Yes. See later in the spec, where this is covered in detail. What is the use case for the includes attribute on locked insertion points? It seems confusing to the user that children can be removed even though the element is locked. Granted, we have to define something, but it seems to me that the simplest solution, i.e. locked means locked and the user is fully in charge of managing the list of inserted children, works just as well. >> 4.4) >> What is the use case for sticky insertion points? >> Aren't locked insertion points enough? >> Additionally, this doesn't match the steps in 4.4.1 > > I've removed the offending paragraph, which was out of date. Sticky insertion points are still there according to the definition of the setInsertionPoint function (first paragraph after the numbered steps). I would recommend adding a first step that says to throw if the element is not locked. I also noticed that step 3 in this list make it sound like the child has to be a DOM Core child of the bound element for the binding. Shouldn't you be allowed to specify the insertion point of any of the explicit children? > In the case of your example with the <h1>, why would you put an <h1> > again in the template? Consider a <button> binding. You wouldn't put a > <button> in the implementation of <button>, right? I can't really come up with a use case for this off the top of my head, we need user experience here I think. >> 4.9.6) >> Why does references in SVG pointing to a bound element use the first >> element in the nodes shadow tree? That seems contrary to other behavior >> where references to a to a bound element still point to the bound >> element, but rendering renders both the bound element and it's >> "flattened children". Maybe this is only desired in SVG when the bound >> element is an SVG element. > > It is so that you can do things like define bindings that define > gradients or fonts or whatnot. It is still weird to me that just the first child is the referenced one. This runs contrary to other uses of XBL where all the children of the <template> are rendered. Would it be possible to say that the bound element should render as a <svg:g> element making the shadow tree under it what is effectively rendered? Or would that extra <svg:g> affect how rendering behaved in undesirable ways? == Comments that are important to me as an implementor == >> 2.13) >> Seems very complicated that <style> elements affect the bound element. >> Which binding to apply, and thus which <style> elements to apply won't >> be known until after style resolution, requiring a second pass. > > Correct. However, it is really required for many use cases. I'll have to run this by our layout folks to see how big the performance hit is going to be. Potentially we won't really know until we implement this part. >> 3) >> "the binding must be applied such that to any running scripts it appears >> that the binding was applied immediately". When is that in relation to >> mutation events? It may be better to say that the binding is applied >> before the mutating function returns. It seems like this only applies to >> element attribute attached bindings? If so it would be good to state >> explicitly > > Yeah, this is something that there are known issues with. I'd actually > really like implementation feedback on this. I've fixed this a bit. "When mutation events are to fire, they must fire after the binding being applied." This can be tricky actually. For example when removing a node from the tree the mutation event is fired before the node is removed. However selector matching would be really hard to do until after the node is removed. So not until then it is possible to know which bindings to add or remove. Somthing like "To mutation events it must look like bindings are applied at the same time as the mutation occurs. This means that mutation events that fire before the mutation will not see the bindings applied, whereas mutation events that fire after the mutation will". This is definitely something that would be good to have tested in the test suite so that we can be sure that implementations pay attention and give feedback for what's hard to do. >> 3.5) >> It is unclear to me if it currently is allowed to call the >> xblBindingAttached/xblEnteredDocument functions after all bindings have >> been constructed and attached for a subtree. This is vital for styling >> performance since we don't want to have to return to the main event loop >> for each bound element during layout. > > I wouldn't attach during layout. I would just make a note of what > bindings need to be applied, and when you have finished your style > pass, apply those bindings and then do layout again. But then it would only be possible to style up to the bound element since before bindings are attached it is impossible to know what children the flattened tree will have. In XUL practically every element has a binding so this would render one element at a time, which of course is unacceptable. >> Why does setting the [xml:base] not cause a mutation event? > > No mutation events must be fired at all during the entire cloning > process, because there didn't seem like a good reason to have them > fire and it would allow performance improvements to block them. Hmm.. Saying that no mutation events at all fire during the above is an interesting idea. However it's only taking us part of the way. At the very least you should say that userdata handlers are not notified of the cloning either. I'm worried that there are other events or notifications that would normally fire that you'd want to avoid firing too. I don't see any other solution than that the spec contain a comprehensive list of which notifications are blocked, but that seems hard or impossible considering future specs. This seems like it could be hard to implement if existing APIs always fire mutation events. It turns out in mozilla that we currently have a flag for not firing them when performing certain mutations, however this is a recent flag, and it's one that I've been wanting to remove. I wonder if this is an optimization that is really doable, though it would be really nice if it was. >> Why the waste of CPU to forward all attributes, including ones that >> haven't changed when any attribute is mutated. > > That's an implementation detail. The spec doesn't require you to > implement this inefficiently if you can think of better ways to do it > than have the same effect. :-) I can see two ways of doing this, both less performant that simply updating the changed attribute. Either you could do the simple thing and update all attributes whenever any of them are changed. Or you would have to keep track of which attributes in the shadow tree have been manually changed and only update those in addition to the changed attribute. Additionally, it seems better for the user to only blow away minimal manual changes to the shadow tree whenever possible. >> 5.2) >> What is the purpose of the xblImplementations property? It seems to >> defeat the purpose of hiding the shadow tree etc in that it can make >> users dependent on the internals of a binding. > > It returns the external object, not the private object. How does it > defeat shadow tree hiding? We should remove the ability to access the implementations by index, otherwise people might do things like .xblImplementations[5].setColor(...) >> Finally I have two concerns without proposed solution: >> >> The browser will behave differently if an element is implemented using >> XBL or using a native implementation when that element is further bound >> by a document author. In one case <inherited> will render content and in >> one it won't. Something that might help this is to let <inherited> in >> the base binding render the contents of the remaining (so far without >> insertion point) children. I.e. let there be an implicit binding with >> the shadow tree <template><children/></template>. > > I intend at some point to work on a spec that defines how various > elements should appear to be implemented at the UA level in terms of > XBL. Fair enough. Another thing I realized this shows up in is the .baseBinding property. It would be good if we for the least derived binding made that return the element itself to make a browser that implement an element using xbl and a browser implement an element using native code work as well as possible with the same binding. == Comments that are less important to me as an implementor == >> Is it really a good idea to implicitly apply bindings into the >> document in which they live? This seems wasteful at the least. Might >> be better to only do this when the <xbl> is not the root element? >> Even then you could require an xbl PI containing just a fragment >> identifier. A use case might be if want to override the look of <h1> >> by wrapping some elements around it. You could do that using a >> template like <template><div class=...><div >> class=...><h1><content/></h1></div></div></template>. However that >> would create a recursive situation if the binding applied to the >> <h1> in the shadow tree. > > I could see an argument for special-casing <xbl> being the root > element, but that seems like a can of worms that we'd want to avoid. If you want to avoid the inconsistency you could say that bindings are never implicitly applied. If you want to put the bindings in the same document as your main document you can do something like: <?xbl href="#myBindings"?> This is mostly a resource saving thing for me. It's unneccesary to have a bunch of bindings imported into a document if you're not going to use them anyway. Worst case I can always add a mozilla specific attribute to the <xbl> element that removes the implicit import if it turns out that it makes a differance. >> 3.9) >> What happens if a binding document has already started loaded and the >> user then calls loadBindingDocument with the same URI? > > See 8.1.1. My question was, what happens if the load has started, but not yet finished? Does loadBindingDocument wait with returning until the load finishes? If not, what does it return since there may not yet be a document created. >> 4) >> Wouldn't having more than one template element in a binding make the >> binding in error? > > Yes, this is already defined. "The template element used to generate a binding is always the first such element in a binding element" says that even if there are multiple <template> elements the binding is still applied. The "in error" section says that elements that are in error are typically ignored, however that conflicts with the above sentence since it says to use the first <template>. >> Why not insert an explicit child text child under the element when >> xbl:text is on the left hand side of the '='. That is more in line >> with how attributes are explicitly mutated when on the left hand >> side of the '='. It also makes it clearer and more easily >> implementable how the inserted child interacts with bindings on the >> element. > > Because the element may have other children in the Core DOM that we > wouldn't want to blow away. Attribute values are blown away the same way though. Note that this doesn't change the bound document, it's just the shadow tree that would get automatically mutated when explicitly requesting it. >> It is in some cases needed that one binding can get to the internal >> implementation object of another binding. One example of this is a >> tab-strip implementation where the binding for the <tabstrip> >> element needs to get to the internals of the individual <tab> >> bindings. One way to do this would be to make it possible for >> bindings from the same binding document to using some API be able to >> reach each others internal implementation object. > > Why can't the <tabstrip> just use the <tab>'s official API? That would force you to put a method like 'showAsSelected' in the public api of the <tab>, which would allow users to make multiple tabs be shown as selected. Basically what I'm asking for is something similar as C++s 'friend' concept. However at this time I don't have a proposal for this, so I'm willing to wait until we have more implementation and usage experience before figuring this out. It is even something that could wait to a later version of the spec. >> 6.2) >> What does the sentence "However, their being in error does not affect >> the processing model described above" mean? Should addEventListenerNS >> still be called? If so, why? > > Yes. In the case of the event attribute being in error, it makes no > difference. In the case of the phase attribute being in error, it > seemed pointless to have the UA check that the attribute had a correct > value before going on. I guess if addEventListenerNS is called or not is really an internal issue, the user won't see a difference either way, right? >> The fact that explicitly inherited bindings are attached at the same >> time as the base binding forces all referenced binding documents to be >> loaded whenever a binding document is imported, whether the bindings are >> ever used or not. This might be good to state. In fact the imported >> document can not be considered loaded until this is done which means >> that .loadBindingDocument must wait until all inherited binding >> documents have been loaded. > > This is an interesting point. I'm not really sure how this works with > mutations of the binding document, though. I've changed > loadBindingDocument() to require that all dependencies be fetched > (well, not all dependencies; I guess stylesheet @imports and such like > wouldn't actually be found unless needed later... this could be > messy). > > What else needs changing? First of all, please write the part "and any bindings defined by that document must be applied to matching elements in the document that corresponds to this DocumentXBL object" as a separate sentence. It took me 5 or so reads to figure out what "that document" refers to (sounds like the inherited bindings document now). You probably also need to change when the bindings-are-ready counter is reduced, or state that loading of dependent binding documents should start right away causing the bindings-are-ready counter to be increased. I don't see where you say that other dependencies (other than inherited bindings) have to be loaded. Did you mean to say that <style> sheets have to be loaded and such too? I'm not requesting that that be made the case, but it sounded like you were saying that. For mutations that happen after the document is considered loaded obviously can't delay the point when the document is considered loaded. For mutations that happen before, I could see leaving this undefined. Alternatively state that mutations that happen before that point must delay loadBindingDocument from returning, but that seems like it could be very messy to implement. / Jonas
Received on Monday, 26 March 2007 22:57:08 UTC