[Webkit-unassigned] [Bug 76553] min-width is not implemented on <table>

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=76553





--- Comment #5 from Max Vujovic <mvujovic at adobe.com>  2012-01-18 14:58:10 PST ---
(In reply to comment #4)
> (From update of attachment 122972 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=122972&action=review

> > Source/WebCore/ChangeLog:7
> > +        min-width does not work on <table>
> > +        https://bugs.webkit.org/show_bug.cgi?id=76553
> > +
> 
> Please go a bit more into detail when implementing new features. What did you implement, how did you implement it. For bigger patches a line by line comment next to the changed functions in the ChangeLog might help as well.

Sure, will do.

> 
> > Source/WebCore/rendering/RenderTable.cpp:273
> > +    bool isCSSTable = !node() || !node()->hasTagName(tableTag);
> > +    if (isCSSTable && (styleWidth.isFixed() && styleWidth.isPositive())) {
> 
> At first, no need to add a new boolean, just put it in the if clause. Can this renderer exist without its node? I don't think that this check is necessary. Do other elements than the HTMLTableElement access RenderTable as well? If not, you dan't need this check at all.

This check addresses the case of an element besides table, such as a div, that has "display: table;" set. We're calling this a "CSS table," and a we're calling an actual table element an "HTML table". In both cases, a RenderTable will be created for the element. We need to differentiate between the cases because width styles on HTML tables include borders and padding, but width styles on CSS tables do not include borders and paddings, so we must include them in the width calculation.

As for the boolean, I think it makes the conditional expression easier to read, but I'll go ahead and remove it since based on our offline discussion, it does not conform to WebKit coding style.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.