Bug 92389 - Outline is always painted on the first table row regardless of the row it's set on
Summary: Outline is always painted on the first table row regardless of the row it's s...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tables (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pravin D
URL:
Keywords: EasyFix, HasReduction
: 94007 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-07-26 09:28 PDT by Julien Chaffraix
Modified: 2012-08-14 13:10 PDT (History)
8 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Chaffraix 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.
Comment 1 Pravin D 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...
Comment 2 Pravin D 2012-07-26 12:19:38 PDT
Created attachment 154716 [details]
Patch
Comment 3 Pravin D 2012-07-26 12:30:55 PDT
Created attachment 154720 [details]
Patch
Comment 4 Eric Seidel (no email) 2012-07-26 12:38:04 PDT
Comment on attachment 154720 [details]
Patch

OK.  LGTM.
Comment 5 Pravin D 2012-07-26 12:38:54 PDT
(In reply to comment #4)
> (From update of attachment 154720 [details])
> OK.  LGTM.
> 

Thnks... :)
Comment 6 Pravin D 2012-07-26 12:39:49 PDT
Hope the pixel test do not cause any problem...
Comment 7 WebKit Review Bot 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>
Comment 8 WebKit Review Bot 2012-07-26 14:19:19 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Julien Chaffraix 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.
Comment 10 Andrew Wilson 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.
Comment 11 Shezan Baig 2012-08-14 13:10:51 PDT
*** Bug 94007 has been marked as a duplicate of this bug. ***