Clean up computePreferredLogicalWidths functions in TableLayout subclasses
Created attachment 189606 [details] Patch
Does this work with bug 109462? Maybe Julien should review this patch :)
(In reply to comment #2) > Does this work with bug 109462? Maybe Julien should review this patch :) As best I can tell, the two patches are totally orthogonal.
Also, this patch does not cause any change in behavior. It's just moving code around.
Comment on attachment 189606 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189606&action=review > Source/WebCore/rendering/AutoTableLayout.cpp:261 > - Length tableLogicalWidth = m_table->style()->logicalWidth(); > - if (tableLogicalWidth.isFixed() && tableLogicalWidth.isPositive()) { > - minWidth = max<int>(minWidth, tableLogicalWidth.value()); > - maxWidth = minWidth; > - } else if (!remainingPercent && maxNonPercent) { > - // if there was no remaining percent, maxWidth is invalid > + // if there was no remaining percent, maxWidth is invalid > + if (!remainingPercent && maxNonPercent) > maxWidth = tableMaxWidth; > - } > + > + Length tableLogicalWidth = m_table->style()->logicalWidth(); > + if (tableLogicalWidth.isFixed() && tableLogicalWidth.isPositive()) > + minWidth = maxWidth = max<int>(minWidth, tableLogicalWidth.value()); I would revert this section of the patch. It doesn't seem equally readable and it's slower. > Source/WebCore/rendering/FixedTableLayout.cpp:82 > + // FIXME: we might want to wait until we have all of the first row before computing for the first time. Nit: Capitalize the first letter of the sentence.
Committed r143683: <http://trac.webkit.org/changeset/143683>
Comment on attachment 189606 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189606&action=review The change is good, sorry for the delay. (In reply to comment #3) > (In reply to comment #2) > > Does this work with bug 109462? Maybe Julien should review this patch :) > > As best I can tell, the two patches are totally orthogonal. They are indeed. However you make the assumption that r130774 which cause bug 109462 is there to cover your back and properly apply min-width / max-width to the result of computePreferredLogicalWidths thus making it difficult to revert the change. > Source/WebCore/rendering/FixedTableLayout.cpp:182 > + minWidth = maxWidth = calcWidthArray() + bordersPaddingAndSpacing; WebKit style is one assignment per line (though I couldn't find an entry in our coding style). That would make this line a lot more readable IMO. > Source/WebCore/rendering/FixedTableLayout.cpp:186 > + minWidth = maxWidth = max<int>(minWidth, tableLogicalWidth.value() - bordersPaddingAndSpacing); Ditto.
(In reply to comment #7) > > Source/WebCore/rendering/FixedTableLayout.cpp:182 > > + minWidth = maxWidth = calcWidthArray() + bordersPaddingAndSpacing; > > WebKit style is one assignment per line (though I couldn't find an entry in our coding style). That would make this line a lot more readable IMO. Discussed this on bug 110520.