REGRESSION(r56655): background-image does not cover the full area of <td> with rowspan > 1

• Component: Tables
• Status: NEW ---

[Daniel Bates]

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>.

[Daniel Bates]

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.

[Beth Dakin]

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.

[Daniel Bates]

(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.

[Daniel Bates]

Comment on attachment 52602 [details]
Patch with test cases

Clearing review flag on this for now to further look into this.

[Daniel Bates]

Rollout of change set 56655 committed in change set 57287
<http://trac.webkit.org/changeset/57287>. See bug #9268 for more details.

[Ahmad Saleem]

Test Case from Patch - table-col-background-cell-colspan.html - https://jsfiddle.net/cmhy9z3w/

^All browsers (Chrome Canary 107, Firefox Nightly 106 and Safari Technology Preview 152) render this fine and don't show text "FAIL" but rather Blue and Yellow blocks or square side by side.

Test Case from Patch - table-row-background-cell-rowspan.html - https://jsfiddle.net/cmhy9z3w/1/

^All browsers (Chrome Canary 107, Firefox Nightly 106 and Safari Technology Preview 152) render this fine and don't show text "FAIL" but rather Blue and Yellow blocks or square above each other.

_______

Is this patch still required? Thanks!