Bug 96710

Summary:

The collapsing border code needs direction-aware border getters

Product:

WebKit

Reporter:

Julien Chaffraix <jchaffraix>

Component:

Tables

Assignee:

Julien Chaffraix <jchaffraix>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bdakin, eric, hyatt, robert, shezbaig.wk, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

79272

Attachments:

Description	Flags
Proposed refactoring. Introduce several new helper functions.	none
Proposed change 2. Forgot to update the column code too.	none
v3: Fixed Mac build, removed unused parameter in RenderTableCol functions.	none

Julien Chaffraix

Reported 2012-09-13 17:22:11 PDT

Currently most of the code makes the assumption that cells have the same alignment as their section. This is wrong and caused bug 79272. Example from RenderTableCell::computeCollapsedStartBorder: CollapsedBorderValue prevCellBorder = CollapsedBorderValue(prevCell->style()->borderEnd(), includeColor ? prevCell->style()->visitedDependentColor(endColorProperty) : Color(), BCELL); This assumes that |prevCell| has the same direction as |this| or this will pick up the wrong border. The idea of this refactoring is to introduce some new direction-aware function to get adjoining borders.

Attachments
Proposed refactoring. Introduce several new helper functions. (14.95 KB, patch) 2012-09-14 17:17 PDT, Julien Chaffraix	no flags	Details Formatted Diff Diff
Proposed change 2. Forgot to update the column code too. (21.93 KB, patch) 2012-09-18 14:20 PDT, Julien Chaffraix	no flags	Details Formatted Diff Diff
v3: Fixed Mac build, removed unused parameter in RenderTableCol functions. (21.98 KB, patch) 2012-09-19 10:51 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-14 17:17:21 PDT

Created attachment 164253 [details] Proposed refactoring. Introduce several new helper functions.

Julien Chaffraix

Comment 2 2012-09-18 14:20:51 PDT

Created attachment 164620 [details] Proposed change 2. Forgot to update the column code too.

Build Bot

Comment 3 2012-09-18 15:42:45 PDT

Comment on attachment 164620 [details] Proposed change 2. Forgot to update the column code too. Attachment 164620 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13897299

Julien Chaffraix

Comment 4 2012-09-19 10:51:53 PDT

Created attachment 164756 [details] v3: Fixed Mac build, removed unused parameter in RenderTableCol functions.

Ojan Vafai

Comment 5 2012-09-19 14:09:53 PDT

Comment on attachment 164756 [details] v3: Fixed Mac build, removed unused parameter in RenderTableCol functions. I find the names all very similar, but different in subtle ways. I couldn't think of any better names though. :(

Julien Chaffraix

Comment 6 2012-09-19 17:47:35 PDT

Comment on attachment 164756 [details] v3: Fixed Mac build, removed unused parameter in RenderTableCol functions. Thanks Ojan! The Qt bots are b0rken, based on previous patches not breaking Qt, this patch should be fine to land.

WebKit Review Bot

Comment 7 2012-09-19 17:58:36 PDT

Comment on attachment 164756 [details] v3: Fixed Mac build, removed unused parameter in RenderTableCol functions. Clearing flags on attachment: 164756 Committed r129078: <http://trac.webkit.org/changeset/129078>

WebKit Review Bot

Comment 8 2012-09-19 17:58:40 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.