Bug 37075 - REGRESSION(r56655): background-image does not cover the full area of <td> with rowspan > 1
Summary: REGRESSION(r56655): background-image does not cover the full area of <td> wit...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tables (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Daniel Bates
URL: http://trac.webkit.org/export/57063/t...
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-04 23:21 PDT by Daniel Bates
Modified: 2010-06-11 10:44 PDT (History)
13 users (show)

See Also:


Attachments
Patch with test cases (96.91 KB, patch)
2010-04-05 21:01 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2010-04-04 23:21:50 PDT
Following the patch for bug #9268, the background-image of a <td> with a rowspan > 1 does not cover its full area as per (5) of section 17.5.1 of the CSS 2.1 specification, <http://www.w3.org/TR/CSS2/tables.html#table-layers>.
Comment 1 Daniel Bates 2010-04-05 21:01:53 PDT
Created attachment 52602 [details]
Patch with test cases

This patch fixes the rowspan issue as observed in the <http://trac.webkit.org/export/57063/trunk/LayoutTests/tables/mozilla/marvin/backgr_simple-table-row.html>.

There are some minor positioning issues observed in the mozilla and mozilla_expected_failure tests (see LayoutTests/table) when compared to Firefox and WebKit before the patch for bug #9268. I am still looking over the CSS 2.1 spec. with regards to background positioning and will consult Beth Dakin and Ian Hickson with regards to this matter.

So as to not bloat this patch with the rebased mozilla, mozilla_expected_failurer tests, I left them out. I suggest we create a separate bug for this.
Comment 2 Beth Dakin 2010-04-08 11:27:13 PDT
I don't think that this patch achieves the desired goal. It looks like tables/mozilla/marvin/backgr_simple-table-row.html is still *slightly* different from Firefox, though it is very slight. (If you follow the black, grey, yellow, and cyan horizontal stripes down one of the edges, you will see that they get out of synch.) Furthermore, a number of the other tests in this directory are still failing with this patch applied. For just a few examples (there are more), see:

tables/mozilla/marvin/backgr_simple-table-column-group.html
tables/mozilla/marvin/backgr_simple-table-column.html
tables/mozilla/marvin/backgr_simple-table-row-group.html

Ultimately, it doesn't look like this patch fixes the regressions caused by the original change. I think we need to consider rolling out the original change because we shouldn't keep regressions in the tree for this long.
Comment 3 Daniel Bates 2010-04-08 11:32:59 PDT
(In reply to comment #2)
> I don't think that this patch achieves the desired goal. It looks like
> tables/mozilla/marvin/backgr_simple-table-row.html is still *slightly*
> different from Firefox, though it is very slight. (If you follow the black,
> grey, yellow, and cyan horizontal stripes down one of the edges, you will see
> that they get out of synch.) Furthermore, a number of the other tests in this
> directory are still failing with this patch applied. For just a few examples
> (there are more), see:
> 
> tables/mozilla/marvin/backgr_simple-table-column-group.html
> tables/mozilla/marvin/backgr_simple-table-column.html
> tables/mozilla/marvin/backgr_simple-table-row-group.html
> 
> Ultimately, it doesn't look like this patch fixes the regressions caused by the
> original change. I think we need to consider rolling out the original change
> because we shouldn't keep regressions in the tree for this long.

Ok, will rollout.
Comment 4 Daniel Bates 2010-04-08 11:33:29 PDT
Comment on attachment 52602 [details]
Patch with test cases

Clearing review flag on this for now to further look into this.
Comment 5 Daniel Bates 2010-04-08 12:24:38 PDT
Rollout of change set 56655 committed in change set 57287
<http://trac.webkit.org/changeset/57287>. See bug #9268 for more details.