Bug 97024

Summary:

Remove isStartColumn in the border collapsing code

Product:

WebKit

Reporter:

Julien Chaffraix <jchaffraix>

Component:

Tables

Assignee:

Julien Chaffraix <jchaffraix>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dglazkov, eric, inferno, pravind.2k4, robert, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Proposed change: remove isEndColumn / isStartColumn and clean up the code.	none
Change v2. Keep isEndColumn and explain why it's needed.	none
Patch for landing.	none

Julien Chaffraix

Reported 2012-09-18 09:29:10 PDT

Those 2 booleans are duplicating the information stored in prevCell / nextCell. As we have to compute the previous / next cell in the common case, it's probably better to stick with them. Patch forthcoming.

Attachments
Proposed change: remove isEndColumn / isStartColumn and clean up the code. (7.03 KB, patch) 2012-09-18 10:02 PDT, Julien Chaffraix	no flags	Details Formatted Diff Diff
Change v2. Keep isEndColumn and explain why it's needed. (6.75 KB, patch) 2012-09-19 10:33 PDT, Julien Chaffraix	no flags	Details Formatted Diff Diff
Patch for landing. (6.89 KB, patch) 2012-09-19 19:00 PDT, Julien Chaffraix	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Julien Chaffraix

Comment 1 2012-09-18 10:02:50 PDT

Created attachment 164580 [details] Proposed change: remove isEndColumn / isStartColumn and clean up the code.

Julien Chaffraix

Comment 2 2012-09-18 15:57:08 PDT

Comment on attachment 164580 [details] Proposed change: remove isEndColumn / isStartColumn and clean up the code. The cr-linux EWS is sick but I confirm that it regresses fast/repaint/table-collapsed-border.html. Holding the patch until I can investigate.

WebKit Review Bot

Comment 3 2012-09-18 21:06:44 PDT

Comment on attachment 164580 [details] Proposed change: remove isEndColumn / isStartColumn and clean up the code. Attachment 164580 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13896379 New failing tests: fast/repaint/table-collapsed-border.html

Julien Chaffraix

Comment 4 2012-09-19 09:41:13 PDT

> The cr-linux EWS is sick but I confirm that it regresses fast/repaint/table-collapsed-border.html. Holding the patch until I can investigate. OK, one of the assumption I made is wrong: isEndColumn == !cellAfter is just untrue because the table doesn't have to be regular in structure. <table> <tr> <td></td> <td></td> </tr> <tr> <td></td> </tr> </table> The previous table is valid but we would wrongly computes the second row's cell's end border.

Julien Chaffraix

Comment 5 2012-09-19 10:12:55 PDT

We need isEndColumn for the previous reason but isStartColumn is fully embedded into cellBefore as we add the cells in order. Renaming the bug.

Julien Chaffraix

Comment 6 2012-09-19 10:33:46 PDT

Created attachment 164754 [details] Change v2. Keep isEndColumn and explain why it's needed.

Julien Chaffraix

Comment 7 2012-09-19 19:00:13 PDT

Created attachment 164822 [details] Patch for landing.

WebKit Review Bot

Comment 8 2012-09-20 08:05:08 PDT

Comment on attachment 164822 [details] Patch for landing. Clearing flags on attachment: 164822 Committed r129136: <http://trac.webkit.org/changeset/129136>

WebKit Review Bot

Comment 9 2012-09-20 08:05:12 PDT

All reviewed patches have been landed. Closing bug.

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