| Summary: | [XBL] Add more tags | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Julien Chaffraix <jchaffraix> |
| Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | ||
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | All | ||
| OS: | All | ||
| Attachments: | |||
|
Description
Julien Chaffraix
2008-07-13 15:57:55 PDT
Created attachment 22262 [details]
Add <binding>, <template>, <div> and <content> stubs
I don't understand what the <div> element is in this example... XBL doesn't have a <div> element does it? (In reply to comment #2) > I don't understand what the <div> element is in this example... XBL doesn't > have a <div> element does it? > It does (http://www.w3.org/TR/xbl/#the-div). It is meant to be a styling hook and should be rendered as a block (http://www.w3.org/TR/xbl/#xbl-elements). Right, but it has no DOM interface does it? I don't see why you would need a special subclass just for a generic block. (In reply to comment #4) > Right, but it has no DOM interface does it? You are right. > I don't see why you would need a > special subclass just for a generic block. We could just use XBLElement to generate the <div> but there is a few drawbacks: - make_names.pl currently creates a wrapper for each tags in the factory files which requires a class per tag. We would have to add an option to generate only the qualified name. - XBL elements have to check their hierarchy to determine if they are in error, I had though of doing it per element. The two previous points are quite simple to solve so either solution is fine. I do not think you should create extra element classes if the element is not going to have any specialized behavior. If you look at HTML, many of the elements do not have any specialized subclasses. Comment on attachment 22262 [details]
Add <binding>, <template>, <div> and <content> stubs
As Hyatt says, any element which does not require custom behavior does not need to be generated for the factory (But should be generated as part of the XBLNames file). You might have to add code to the generation scripts for that (But html needs that code anyway... when we finally make HTMLNames and HTMLElementFactory autogen -- there is another bug on that).
If it doesn't need custom behavior, you just test for the element using:
element->hasTagName(XBLNames::divTag)
the generated factory stuff should correctly generate generic XBLElements in that case, since these elements will have the XBL namespace (I know this works for SVG, so I'm sure the generated stuff will do the right hting for XBL too.)
Created attachment 22304 [details]
Updated patch: add the tags and only the useful classes
Created attachment 22341 [details]
Updated: add <binding>, template> and <div> but do not create XBLDivElement class
Comment on attachment 22341 [details]
Updated: add <binding>, template> and <div> but do not create XBLDivElement class
xblCustomMappings shouldn't be needed. HTML will need this too. This should just be a property of the tag in the .in file.
Also, I don't think you need a comment about <div> in the XBLElement header. XBLElements probably want to be StyledElements anyway. I mean, it's also possible to just create an empty XBLDivElement, despite hyatt's suggestions to the contrary. The cost in terms of code and binary size are negligible.
I think you should make the "generate w/o having a custom class" thing work though, since our first pass in converting HTMLElementFactory will use that.
Created attachment 22371 [details] Patch updated with Eric's comments (In reply to comment #11) > (From update of attachment 22341 [details] [edit]) > xblCustomMappings shouldn't be needed. HTML will need this too. This should > just be a property of the tag in the .in file. I thought it would interact badly with the .in file format change discussed on the ML that I intend to do (thus my original FIXME). However after thinking about it, I guess I could add just one property without too much trouble. > Also, I don't think you need a comment about <div> in the XBLElement header. > XBLElements probably want to be StyledElements anyway. I mean, it's also > possible to just create an empty XBLDivElement, despite hyatt's suggestions to > the contrary. The cost in terms of code and binary size are negligible. HTMLElement also derives from StyledElement so we could do the same here too. > I think you should make the "generate w/o having a custom class" thing work > though, since our first pass in converting HTMLElementFactory will use that. It should work. (In reply to comment #11) > (From update of attachment 22341 [details] [edit]) > xblCustomMappings shouldn't be needed. HTML will need this too. This should > just be a property of the tag in the .in file. > > Also, I don't think you need a comment about <div> in the XBLElement header. > XBLElements probably want to be StyledElements anyway. I mean, it's also > possible to just create an empty XBLDivElement, despite hyatt's suggestions to > the contrary. The cost in terms of code and binary size are negligible. > XBLElements should not be StyledElements. XBL is not a language that responds to inline style or to the class attribute, so you would create bugs by inheriting from StyledElement. I will double-check this with Hixie, but I'm pretty sure the XBL div does not support class or style attributes. I also disagree with making XBLDivElement. There is absolutely no reason to need that class. Why generate it? We should not make classes for elements that don't need them. Ok, the <div> element does support class according to the spec (but no mention is made of style). Other XBL elements do not. This makes the hierarchy weird. It means XBLDivElement might need to derive from StyledElement while the other XBLElement don't. Created attachment 22378 [details]
Updated patch: make XBLDivElement inherits from StyledElement
Comment on attachment 22378 [details]
Updated patch: make XBLDivElement inherits from StyledElement
Seems like either all XBLElements should be StyledElements, or you'll need a StyledXBLElement to inherit from.
As discussed on IRC, even if it may lead to bugs we will simplify the hierarchy by having XBLElement inherit from StyledElement (without creating an XBLDivElement class). It is ok for a first-pass and David Hyatt pointed that we should make StyledElement a mix-in to solve this issue. Created attachment 22436 [details]
Repost "Patch updated with Eric's comments" (matches the latest discussion)
Comment on attachment 22436 [details]
Repost "Patch updated with Eric's comments" (matches the latest discussion)
LGTM.
|