Bug 46417 – Make tables work with vertical text

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Dave Hyatt]

Make tables work with vertical text.

[Dave Hyatt]

See https://bugs.webkit.org/show_bug.cgi?id=46442... have to remove that code for tables eventually...

[Adele Peterson]

<rdar://problem/8612061>

Created attachment 72859 [details]
Part 1: make column width allocation algorithms (auto and fixed) work with vertical layout

[Dave Hyatt]

Comment on attachment 72859 [details]
Part 1: make column width allocation algorithms (auto and fixed) work with vertical layout

View in context: https://bugs.webkit.org/attachment.cgi?id=72859&action=review

r=me

> WebCore/rendering/FixedTableLayout.cpp:223
> +    if (m_table->document()->inQuirksMode() && m_table->style()->width().isPercent() && maxWidth < TABLE_MAX_WIDTH)

Should this be m_table->style()->logicalWidth().isPercent()?

> WebCore/rendering/RenderTable.h:115
> -    int bordersPaddingAndSpacing() const
> +    int bordersPaddingAndSpacingInRowDirection() const
>      {
>          return borderLeft() + borderRight() +
>                 (collapseBorders() ? 0 : (paddingLeft() + paddingRight() + (numEffCols() + 1) * hBorderSpacing()));
>  

Did you want to go ahead and patch this or save it for another patch?  It's using left/right border and padding still.

Part 1 landed as <http://trac.webkit.org/projects/webkit/changeset/71262>.

Created attachment 72899 [details]
Part 2: layout and painting

[Dave Hyatt]

Comment on attachment 72899 [details]
Part 2: layout and painting

View in context: https://bugs.webkit.org/attachment.cgi?id=72899&action=review

r=me

> WebCore/rendering/RenderTable.h:60
> +        return style()->isFlippedBlocksWritingMode() ? borderAfter() : borderAfter();

This looks wrong.  One of them should be a borderBefore.

> WebCore/rendering/RenderTable.h:95
> +        return style()->isFlippedBlocksWritingMode() ? outerBorderAfter() : outerBorderAfter();

Same bug here.

Part 2 landed as <http://trac.webkit.org/projects/webkit/changeset/71382>.

Created attachment 73397 [details]
Part 3: hit testing

[Dave Hyatt]

Comment on attachment 73397 [details]
Part 3: hit testing

r=me

[Darin Adler]

Comment on attachment 73397 [details]
Part 3: hit testing

View in context: https://bugs.webkit.org/attachment.cgi?id=73397&action=review

> WebCore/rendering/RenderTableSection.cpp:1245
> +    // Now set hitRow to the index of to hit row, or 0.

This says "index of to hit row", but maybe means "index of the hit row" or "index of the row that was hit"? Maybe using two separate local variable would document this without a comment. The first one would be named something other than hitRow since it’s not the row that was hit yet.

Part 3 landed as <http://trac.webkit.org/projects/webkit/changeset/71668>.

Created attachment 75851 [details]
Part 4: invalidation

[Simon Fraser (smfr)]

Comment on attachment 75851 [details]
Part 4: invalidation

i can haz repaint test?

Created attachment 75852 [details]
Part 4: invalidation

With tests

Split perpendicular and flipped cells off into bug 50658.

Part 4 landed as <http://trac.webkit.org/changeset/73484>.