On Wed, Oct 13, 2010 at 03:13:16PM -0300, Jorge Arellano Cid wrote:
On Tue, Oct 12, 2010 at 09:54:13PM +0200, Johannes Hofmann wrote:
*snip*
Oh, "The collapsing border model" in the spec looks tricky :-)
Given the too many overlapping directives in the model, it was destined to be complex.
Personally I believe the collapsing model and its baroque combinations are seldom used in the Web. Most sites are structured around floating DIVs these days, and even the last SPEC more or less recognizes the problem:
<cite> Automatic table layout algorithm
The input to the suggested (non-normative) automatic layout algorithm for tables is restricted to (1) the containing block width and (2) the content and properties of the table and its children. This restriction may be lifted. </cite>
Nevertheless, digging into the SPEC makes clear that if the collapsing border model is to be partly implemented (the simple cases at least), there's a need to move rendering from the cell widget into the table in Dw.
The only way I've found is to set a hard value in postprocessAttrs(), but not yet a way to do it from Html_tag_open_table().
As you're working on the API, please give me some hints on how it works.
The basic idea is that the stuff in css.cc is doing pure CSS and does not care about the actual implementation or rendering in dw/*. StyleEngine then translates this to dw::core::style::Style objects (which also were designed with CSS in mind).
Yes, that's the basis. The engine also takes care of inheritance, rules, nonCSS hints (i.e. old HTML visual directives) and their precedence.
One specific question is where's the correct point to take into account that cell margin should be ignored in table cells.
One way is to tweak the value (set it to 0), but then the "computed" value is not honoured as you point out.
If the spec would say, that the computed value is set to 0 if border-collapse is 'collapse' (which it doesn't say as far as I can see), the correct place would be in postprocessAttrs(). E.g. in http://www.w3.org/TR/CSS2/box.html#propdef-border-top-width it is stated that "Computed?value:??absolute length; '0' if the border style is 'none' or 'hidden'" That's why borderWidth is set to 0 in postprocessAttrs() if borderStyle is 'none'. ... and now I see, that the same thing for 'hidden' is missing. Boh, I always thought that the difference between 'none' and 'hidden' was exactly that 'hidden' does not affect the computed value of border-width - seems that I was wrong. If the computed value should not be changed, we need to tweak the style somewhere else where it does not interfere with inheritance (see below).
Another is to ignore it in the draw method, but then draw() must know what element it is drawing, and for "collapsing" there's a need to know much more context.
I quite agree with your view of the subject matter.
As currently the cell border, padding and margin are drawn by the cell widget (textblock) inside a table, the general idea is to make textblock consider "collapse" mode when inside a table.
postprocessAttrs() does not take into account in which element we currently are (e.g. <table>, <td>, ...) You could check whether you currently are in a certain element by looking at doctree->top ()->element and do special things for certain elements. However looking at "The collapsing border model" I'm not sure whether you can get away with leaving the dw/* code as it is and just fiddling with the Style objects. For example if you have
<td>1</td><td>2</td>
There currently is no way to get the style of the first <td> when creating the style of the second <td> in StyleEngine - which you would need to compute the thickness of the border between the two.
Fully agreed.
Actually we can't play with the values of the Style object in postprocessAttrs() as we like. They must contain what the spec mentions as "computed value". And as I read the spec, the computed value of e.g. border-width does not depend on the value of border-collapse.
I didn't know this ("computed value" requirement). Well, it defines better what's to be done if ever.
What *might* work is to take the Style object that StyleEngine gives you and modify it in in case that border-collapse is set before passing it to the table cell in src/table.cc:293.
Do you mean as if it was a nonCssHint? I was afraid CSS would take precedence (i.e. if nonCssHint says margin is 0 and CSS says it's 20, I'd bet on the second one!).
No, not via nonCssHints. I meant to directly modify the style before giving it to the Textblock. Attached is a small patch to demonstrate what I mean. With some more work I think we could make it cover the most common cases of the collapsing border model.
At this point, if we can set margin to zero for the "separate" model, and half the value for "collapsing" it would be quite an advance (in rendering). OTOH, it must be commented as a workaround because it's not the correct way to implement it. An implementation would need major surgery.
Another approach is to make dw::table render its borders and margin, and only leave padding to the cell widget.
Yes, that's the other way to do it. You could still reuse the border drawing helper routines from style.cc. Nevertheless it might be pretty complex. Not sure if it is really worth it...
Fully agreed.
Most probably a full implementation is not worth ATM, given the small use cases out there. A partial workaround may help though.
Yes, I think it would be reasonable to just cover the common cases e.g a table with a 1px grid with a single color and style.
- but of course I don't want to discourage you :-)
you did! :-)
Seriously, so far we have a better effort estimation on it, and we know how not to try to implement it, and what a correct implementation would need.
Given the high cost/benefit, besides a workaround, i'd move to investigate 8 and 6:
8 support more border-style options (dotted, dashed ...) Once implemented, this code can be reused/moved if necessary.
6 support more vertical-align options I hope vertical-align in the textblock can help with vertical align inside the table. If rows need to coordinate it will be complex. This feature is much more visible than border-collapse. This needs coordination among us, and corvid has already advanced working on it.
Ok. Cheers, Johannes