RESOLVED FIXED 97024
Remove isStartColumn in the border collapsing code
https://bugs.webkit.org/show_bug.cgi?id=97024
Summary Remove isStartColumn in the border collapsing code
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
Change v2. Keep isEndColumn and explain why it's needed. (6.75 KB, patch)
2012-09-19 10:33 PDT, Julien Chaffraix
no flags
Patch for landing. (6.89 KB, patch)
2012-09-19 19:00 PDT, Julien Chaffraix
no flags
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.