Bug 92389

Summary:

Outline is always painted on the first table row regardless of the row it's set on

Product:

WebKit

Reporter:

Julien Chaffraix <jchaffraix>

Component:

Tables

Assignee:

Pravin D <pravind.2k4>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

atwilson, eae, eric, leviw, pravind.2k4, robert, shezbaig.wk, webkit.review.bot

Priority:

Keywords:

EasyFix, HasReduction

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Test case: both outlines shouldn't be on the first row	none
Patch	none
Patch	none

Julien Chaffraix

Reported 2012-07-26 09:28:50 PDT

Created attachment 154669 [details] Test case: both outlines shouldn't be on the first row This is a super simple bug. We don't account for the row's position when painting the outline and thus always paint it on the first row, regardless of where it is set up. See the attached test case. The issue is in RenderTableRow::paintOutlineForRowIfNeeded that doesn't move the |paintOffset| accordingly.

Attachments
Test case: both outlines shouldn't be on the first row (416 bytes, text/html) 2012-07-26 09:28 PDT, Julien Chaffraix	no flags	Details
Patch (8.75 KB, text/plain) 2012-07-26 12:19 PDT, Pravin D	no flags	Details
Patch (9.60 KB, patch) 2012-07-26 12:30 PDT, Pravin D	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Pravin D

Comment 1 2012-07-26 11:58:44 PDT

(In reply to comment #0) > Created an attachment (id=154669) [details] > Test case: both outlines shouldn't be on the first row > > This is a super simple bug. We don't account for the row's position when painting the outline and thus always paint it on the first row, regardless of where it is set up. See the attached test case. > > The issue is in RenderTableRow::paintOutlineForRowIfNeeded that doesn't move the |paintOffset| accordingly. > Super super simple bug I guess... They have properly handled Table sections n forgot the rows...

Pravin D

Comment 2 2012-07-26 12:19:38 PDT

Created attachment 154716 [details] Patch

Pravin D

Comment 3 2012-07-26 12:30:55 PDT

Created attachment 154720 [details] Patch

Eric Seidel (no email)

Comment 4 2012-07-26 12:38:04 PDT

Comment on attachment 154720 [details] Patch OK. LGTM.

Pravin D

Comment 5 2012-07-26 12:38:54 PDT

(In reply to comment #4) > (From update of attachment 154720 [details]) > OK. LGTM. > Thnks... :)

Pravin D

Comment 6 2012-07-26 12:39:49 PDT

Hope the pixel test do not cause any problem...

WebKit Review Bot

Comment 7 2012-07-26 14:19:15 PDT

Comment on attachment 154720 [details] Patch Clearing flags on attachment: 154720 Committed r123793: <http://trac.webkit.org/changeset/123793>

WebKit Review Bot

Comment 8 2012-07-26 14:19:19 PDT

All reviewed patches have been landed. Closing bug.

Julien Chaffraix

Comment 9 2012-07-26 15:26:42 PDT

Comment on attachment 154720 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154720&action=review > LayoutTests/fast/table/table-row-outline-paint.html:17 > +<table> Descriptions and expectations for maintainers are always nice, even in comments or not visible. > LayoutTests/platform/chromium-linux/fast/table/table-row-outline-paint-expected.txt:1 > +layer at (0,0) size 800x600 The baseline was wrongly added for Chromium only. Without anything tied to a platform, it's a pity it wasn't put next to the test case and shared by all platforms. Hopefully webkit-patch rebaseline will do the right thing here.

Andrew Wilson

Comment 10 2012-07-26 16:10:57 PDT

Added more updated expectations in http://trac.webkit.org/changeset/123812 - please review to make sure they are correct.

Shezan Baig

Comment 11 2012-08-14 13:10:51 PDT

*** Bug 94007 has been marked as a duplicate of this bug. ***

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