Bug 46417

Summary:

Make tables work with vertical text

Product:

WebKit

Reporter:

Dave Hyatt <hyatt>

Component:

Layout and Rendering

Assignee:

mitz

Status:

RESOLVED FIXED

Severity:

Normal

CC:

mitz

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Bug Depends on:

50658, 46191

Bug Blocks:

46123

Attachments:

Description	Flags
Part 1: make column width allocation algorithms (auto and fixed) work with vertical layout	hyatt: review+
Part 2: layout and painting	hyatt: review+
Part 3: hit testing	hyatt: review+
Part 4: invalidation	none
Part 4: invalidation	darin: review+

Dave Hyatt

Reported 2010-09-23 15:25:41 PDT

Make tables work with vertical text.

Attachments
Part 1: make column width allocation algorithms (auto and fixed) work with vertical layout (61.68 KB, patch) 2010-11-03 13:28 PDT, mitz	hyatt: review+	Details Formatted Diff Diff
Part 2: layout and painting (1.18 MB, patch) 2010-11-03 19:51 PDT, mitz	hyatt: review+	Details Formatted Diff Diff
Part 3: hit testing (13.93 KB, patch) 2010-11-09 12:04 PST, mitz	hyatt: review+	Details Formatted Diff Diff
Part 4: invalidation (4.65 KB, patch) 2010-12-07 17:04 PST, mitz	no flags	Details Formatted Diff Diff
Part 4: invalidation (45.68 KB, patch) 2010-12-07 17:21 PST, mitz	darin: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Dave Hyatt

Comment 1 2010-09-23 19:21:29 PDT

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

Adele Peterson

Comment 2 2010-10-29 15:14:51 PDT

<rdar://problem/8612061>

mitz

Comment 3 2010-11-03 13:28:59 PDT

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

Dave Hyatt

Comment 4 2010-11-03 13:39:35 PDT

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.

mitz

Comment 5 2010-11-03 13:45:23 PDT

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

mitz

Comment 6 2010-11-03 19:51:57 PDT

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

Dave Hyatt

Comment 7 2010-11-04 14:19:52 PDT

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.

mitz

Comment 8 2010-11-04 19:44:30 PDT

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

mitz

Comment 9 2010-11-09 12:04:17 PST

Created attachment 73397 [details] Part 3: hit testing

Dave Hyatt

Comment 10 2010-11-09 12:07:10 PST

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

Darin Adler

Comment 11 2010-11-09 12:08:10 PST

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.

mitz

Comment 12 2010-11-09 12:46:44 PST

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

mitz

Comment 13 2010-12-07 17:04:30 PST

Created attachment 75851 [details] Part 4: invalidation

Simon Fraser (smfr)

Comment 14 2010-12-07 17:06:57 PST

Comment on attachment 75851 [details] Part 4: invalidation i can haz repaint test?

mitz

Comment 15 2010-12-07 17:21:01 PST

Created attachment 75852 [details] Part 4: invalidation With tests

mitz

Comment 16 2010-12-07 17:25:55 PST

Split perpendicular and flipped cells off into bug 50658.

mitz

Comment 17 2010-12-07 17:39:21 PST

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

Note You need to log in before you can comment on or make changes to this bug.