WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
71244
CSS 2.1 failure: border-conflict-element-*
https://bugs.webkit.org/show_bug.cgi?id=71244
Summary
CSS 2.1 failure: border-conflict-element-*
Robert Hogan
Reported
2011-10-31 15:25:12 PDT
Several failures in this group of tests.
Attachments
Patch
(903.24 KB, patch)
2011-11-01 13:53 PDT
,
Robert Hogan
no flags
Details
Formatted Diff
Diff
Patch
(1.29 MB, patch)
2011-11-06 08:57 PST
,
Robert Hogan
no flags
Details
Formatted Diff
Diff
Patch
(1.28 MB, patch)
2011-11-07 13:57 PST
,
Robert Hogan
no flags
Details
Formatted Diff
Diff
Patch
(1.28 MB, patch)
2011-11-16 13:32 PST
,
Robert Hogan
no flags
Details
Formatted Diff
Diff
Patch
(1.28 MB, patch)
2011-11-18 13:31 PST
,
Robert Hogan
no flags
Details
Formatted Diff
Diff
Patch
(1.28 MB, patch)
2011-11-20 04:38 PST
,
Robert Hogan
no flags
Details
Formatted Diff
Diff
Patch
(1.32 MB, patch)
2011-12-04 09:11 PST
,
Robert Hogan
no flags
Details
Formatted Diff
Diff
Patch
(943.10 KB, patch)
2011-12-07 13:30 PST
,
Robert Hogan
no flags
Details
Formatted Diff
Diff
Patch
(800.71 KB, patch)
2011-12-09 12:22 PST
,
Robert Hogan
darin
: review+
Details
Formatted Diff
Diff
Show Obsolete
(8)
View All
Add attachment
proposed patch, testcase, etc.
Robert Hogan
Comment 1
2011-11-01 13:53:28 PDT
Created
attachment 113209
[details]
Patch
Robert Hogan
Comment 2
2011-11-06 08:57:47 PST
Created
attachment 113784
[details]
Patch
Robert Hogan
Comment 3
2011-11-07 13:57:59 PST
Created
attachment 113924
[details]
Patch
Dave Hyatt
Comment 4
2011-11-09 10:42:54 PST
Comment on
attachment 113924
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=113924&action=review
> Source/WebCore/rendering/style/CollapsedBorderValue.h:44 > + , m_offsetFromStart(offsetFromStart) > + , m_offsetFromTop(offsetFromTop)
Why not call these row and column offsets? Seems weird to use the terms offsetFromStart and offsetFromTop when you just mean row and column position.
> Source/WebCore/rendering/style/CollapsedBorderValue.h:79 > + int m_offsetFromStart; > + int m_offsetFromTop;
Shouldn't these be unsigned?
Robert Hogan
Comment 5
2011-11-16 13:32:02 PST
Created
attachment 115437
[details]
Patch
Julien Chaffraix
Comment 6
2011-11-17 14:57:37 PST
Comment on
attachment 115437
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=115437&action=review
r- mostly for the int <-> unsigned issues. Also some inconsistencies.
> Source/WebCore/rendering/RenderTableCell.cpp:456 > colElt = table->colElement(col() -1, &startColEdge, &endColEdge);
Nit: space between '-' and '1'.
> Source/WebCore/rendering/RenderTableCell.cpp:484 > + RenderTableCell* nextCell = table->cellAfter(this);
That doesn't look like a good change as you don't use nextCell outside the if.
> Source/WebCore/rendering/RenderTableCell.cpp:572 > + result = chooseBorder(result, CollapsedBorderValue(prevRow->style()->borderAfter(), prevRow->style()->visitedDependentColor(after), BROW, col(), prevCell->row(), BAFTEREDGE));
Shouldn't it be prevCell->col() here for consistency?
> Source/WebCore/rendering/RenderTableCell.cpp:582 > + result = chooseBorder(result, CollapsedBorderValue(currSection->style()->borderBefore(), currSection->style()->visitedDependentColor(before), BROWGROUP, col(), row(), BBEFOREEDGE));
Shouldn't you add the rowSpan() here to match section (6)?
> Source/WebCore/rendering/RenderTableCell.cpp:589 > + result = chooseBorder(CollapsedBorderValue(currSection->style()->borderAfter(), currSection->style()->visitedDependentColor(after), BROWGROUP, col(), row() - rowSpan(), BAFTEREDGE), result);
Nothing ensures that row() > rowSpan() here. This sounds like you could underflow your unsigned.
> Source/WebCore/rendering/RenderTableCell.cpp:643 > + result = chooseBorder(result, CollapsedBorderValue(nextCell->parent()->style()->borderBefore(), nextCell->parent()->style()->visitedDependentColor(before), BROW, col(), nextCell->row(), BBEFOREEDGE));
Again shouldn't it be nextCell->col() for consistency?
> Source/WebCore/rendering/style/CollapsedBorderValue.h:39 > + CollapsedBorderValue(const BorderValue& b, Color c, EBorderPrecedence p, int rowOffset, int columnOffset, EEdgePrecedence edge)
Those should be unsigned too.
> Source/WebCore/rendering/style/CollapsedBorderValue.h:66 > + int rowOffset() const { return m_rowOffset; }
Those 2 should return an unsigned too.
> Source/WebCore/rendering/style/CollapsedBorderValue.h:79 > + unsigned int m_columnOffset;
Unsigned (not int) is the preferred usage in WebKit.
> Source/WebCore/rendering/style/RenderStyleConstants.h:101 > +enum EEdgePrecedence { BAFTEREDGE, BENDEDGE, BBEFOREEDGE, BSTARTEDGE};
Space before the final }
Robert Hogan
Comment 7
2011-11-18 13:25:20 PST
Comment on
attachment 115437
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=115437&action=review
>> Source/WebCore/rendering/RenderTableCell.cpp:582 >> + result = chooseBorder(result, CollapsedBorderValue(currSection->style()->borderBefore(), currSection->style()->visitedDependentColor(before), BROWGROUP, col(), row(), BBEFOREEDGE)); > > Shouldn't you add the rowSpan() here to match section (6)?
I could, since all that matters is that section 6 is given an offset nearer the top than the one here. See next comment.
>> Source/WebCore/rendering/RenderTableCell.cpp:589 >> + result = chooseBorder(CollapsedBorderValue(currSection->style()->borderAfter(), currSection->style()->visitedDependentColor(after), BROWGROUP, col(), row() - rowSpan(), BAFTEREDGE), result); > > Nothing ensures that row() > rowSpan() here. This sounds like you could underflow your unsigned.
All that matters is that the after border of a row group above the current one is given an offset nearer the top of the table than the offset given the current one. This is so that its after border will always beat the current row group's before border if there's nothing else between them. So I could just use row + rowSpan() for (5) and row() for (6) - it would have the same effect and allow me to keep the ints unsigned. I guess that's the way I'll go.
Robert Hogan
Comment 8
2011-11-18 13:31:18 PST
Created
attachment 115860
[details]
Patch
Robert Hogan
Comment 9
2011-11-18 13:33:08 PST
(In reply to
comment #6
)
> (From update of
attachment 115437
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=115437&action=review
> > r- mostly for the int <-> unsigned issues. Also some inconsistencies. >
Thanks! Hopefully I addressed all of these in the updated patch.
Julien Chaffraix
Comment 10
2011-11-19 08:05:14 PST
Comment on
attachment 115860
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=115860&action=review
> Source/WebCore/rendering/RenderTableCell.cpp:388 > + // the one further to the top wins
Nit: comments starts wtih a capital letters and ends with a '.'
> Source/WebCore/rendering/RenderTableCell.cpp:392 > + // the one further to the left if 'ltr' and right if 'rtl' and to the top wins
Ditto.
> Source/WebCore/rendering/RenderTableCell.cpp:523 > + CollapsedBorderValue startBorder = CollapsedBorderValue(colElt->style()->borderStart(), colElt->style()->visitedDependentColor(start), BCOL, col() + colSpan(), row(), BSTARTEDGE);
> All that matters is that the after border of a row group above the current one is given an offset nearer the top of the table than the offset given the current one. This is so that its after border will always beat the current row group's before border if there's nothing else between them. > So I could just use row + rowSpan() for (5) and row() for (6) - it would have the same effect and allow me to keep the ints unsigned. I guess that's the way I'll go.
There is something wrong here: if you underflow an unsigned, you end up with a huge value that wouldn't match what you were just saying here. Or at least this new code is not totally equivalent and yet you don't mention any test that was failing or needing a rebaseline. If I am right, it looks like we would need a simple test case for the underflow case.
> Source/WebCore/rendering/style/CollapsedBorderValue.h:44 > + , m_columnOffset(columnOffset)
I am still not a huge fan to call those 'offsets' as they are just the row and column (apart from one case where one represents the columnAfter) but I could live with it.
WebKit Review Bot
Comment 11
2011-11-19 16:12:05 PST
Comment on
attachment 115860
[details]
Patch
Attachment 115860
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/10462160
New failing tests: css2.1/20110323/abspos-containing-block-initial-004b.htm css2.1/20110323/abspos-containing-block-initial-004d.htm
Robert Hogan
Comment 12
2011-11-20 03:52:14 PST
(In reply to
comment #10
)
> > All that matters is that the after border of a row group above the current one is given an offset nearer the top of the table than the offset given the current one. This is so that its after border will always beat the current row group's before border if there's nothing else between them. > > So I could just use row + rowSpan() for (5) and row() for (6) - it would have the same effect and allow me to keep the ints unsigned. I guess that's the way I'll go. > > There is something wrong here: if you underflow an unsigned, you end up with a huge value that wouldn't match what you were just saying here. Or at least this new code is not totally equivalent and yet you don't mention any test that was failing or needing a rebaseline. If I am right, it looks like we would need a simple test case for the underflow case.
I agree the sequence of patches has made this confusing: The first one (113924) used a signed int so coped with row()- rowSpan() fine - so no underflow. The second one (115437) incompletely moved it to a signed int. The constructor used a signed int, stored it in an unsigned int, but then returned it as signed when CollapsedBorderValue::rowOffset() was queried. So the underflow was temporary and internal to CollapsedBorderValue only. In the final patch (115860) the values are all unsigned, so it is the first patch in which row() - rowSpan() will actually underflow when CollapsedBorderValue::rowOffset() is used. And indeed if I do row() - rowSpan() border-conflict-element-021.htm starts failing. So you're absolutely correct to be suspicious but I think it's just a confusing sequence of patches.
Robert Hogan
Comment 13
2011-11-20 04:32:24 PST
(In reply to
comment #11
)
> (From update of
attachment 115860
[details]
) >
Attachment 115860
[details]
did not pass chromium-ews (chromium-xvfb): > Output:
http://queues.webkit.org/results/10462160
> > New failing tests: > css2.1/20110323/abspos-containing-block-initial-004b.htm > css2.1/20110323/abspos-containing-block-initial-004d.htm
I can't recreate these failures locally.
Robert Hogan
Comment 14
2011-11-20 04:38:51 PST
Created
attachment 115980
[details]
Patch
Julien Chaffraix
Comment 15
2011-11-21 08:03:21 PST
Comment on
attachment 115980
[details]
Patch
> So you're absolutely correct to be suspicious but I think it's just a confusing sequence of patches.
I see your point but it is not only that. The second patch did not seem to make any test go bad but the underflow should have made one case fail as the resolution would have favored the wrong border. I am asking for such a test to be added as it does not seem to be covered in the CSS test suite. The change looks fine but I really would like to see the case covered before r+'ing.
Robert Hogan
Comment 16
2011-11-21 10:52:41 PST
(In reply to
comment #15
)
> (From update of
attachment 115980
[details]
) > > So you're absolutely correct to be suspicious but I think it's just a confusing sequence of patches. > > I see your point but it is not only that. The second patch did not seem to make any test go bad but the underflow should have made one case fail as the resolution would have favored the wrong border. I am asking for such a test to be added as it does not seem to be covered in the CSS test suite.
The reason I gave for no test failing in the second patch was: "The second one (115437) incompletely moved it to a signed int. The constructor used a signed int, stored it in an unsigned int, but then returned it as signed when CollapsedBorderValue::rowOffset() was queried. So the underflow was temporary and internal to CollapsedBorderValue only." If I had correctly implemented the second patch and converted CollapsedBorderValue::rowOffset() to return an unsigned int, border-conflict-element-021.htm would have started failing. My understanding is that casting from signed->unsigned->signed ensured that in the end the underflowed value was *not* used when deciding where in the table the rowgroup's border was. I confirmed this in practice by converting CollapsedBorderValue to use an unsigned in all places for rowOffset, left the row()- rowSpan() in the place you highlighted it, and border-conflict-element-021.htm started failing on me. I also retested patch '2' and change rowOffset to return an unsigned, and it also failed. So border-conflict-element-021.htm tests for the potential regression you spotted and it didn't fail on patch '2' because the logic that checks the position of the border's cell was using a signed value rather than an unsigned.
Julien Chaffraix
Comment 17
2011-11-22 15:37:17 PST
Comment on
attachment 115980
[details]
Patch
> If I had correctly implemented the second patch and converted CollapsedBorderValue::rowOffset() to return an unsigned int, border-conflict-element-021.htm would have started failing. My understanding is that casting from signed->unsigned->signed ensured that in the end the underflowed value was *not* used when deciding where in the table the rowgroup's border was.
My bad, I missed the double signed <-> unsigned cast which would cancel any visible change in the resolution.
> So border-conflict-element-021.htm tests for the potential regression you spotted and it didn't fail on patch '2' because the logic that checks the position of the border's cell was using a signed value rather than an unsigned.
Thanks for double checking, appreciated!
Robert Hogan
Comment 18
2011-11-27 10:34:05 PST
Committed
r101202
: <
http://trac.webkit.org/changeset/101202
>
Mark Rowe (bdash)
Comment 19
2011-11-28 12:31:14 PST
This appears to have completely hosed performance on some pages. See
bug 73238
for details.
Robert Hogan
Comment 20
2011-11-28 15:12:02 PST
Reverted
r101202
for reason: Caused performance regressions when painting collapsed borders Committed
r101292
: <
http://trac.webkit.org/changeset/101292
>
Robert Hogan
Comment 21
2011-12-04 09:11:39 PST
Created
attachment 117792
[details]
Patch
Robert Hogan
Comment 22
2011-12-04 09:20:24 PST
(In reply to
comment #21
)
> Created an attachment (id=117792) [details] > Patch
The reason the original patch created such a performance hit was because it increased the number of unique collapsed border values for a table from the number of unique borders by size, width, and color to the number of cells in the table x 4. Since RenderTable paints all cells in the table for each unique border value this meant every cell in a large table would get repainted thousands of times per table paint. The revised solution is to conform to the spec by changing the order in which cells are painted and avoid treating collapsed borders that only differ in color as unique. See the log for more details. This new approach does not depend on the enhancement proposed in 73349 in order to keep performance. By itself it actually improves performance in tables where there are many collapsed borders that differ only in color, since it will save a full repaint of cells for each different color.
Julien Chaffraix
Comment 23
2011-12-05 15:23:23 PST
Comment on
attachment 117792
[details]
Patch I am pretty sure this will make us paint badly some cases. Think of cases involving a cell with some overflow on the left of it: with your patch, the painting order will be inverted (the overflow will be under the left box's content). You can add outline, box-shadow, negative margins, negative top/left to this example.
Robert Hogan
Comment 24
2011-12-05 15:55:55 PST
(In reply to
comment #23
)
> (From update of
attachment 117792
[details]
) > I am pretty sure this will make us paint badly some cases. Think of cases involving a cell with some overflow on the left of it: with your patch, the painting order will be inverted (the overflow will be under the left box's content). You can add outline, box-shadow, negative margins, negative top/left to this example.
tables/mozilla_expected_failures/marvin/table_overflow_* seem to cover this kind of thing. One of them is rebaselined in this patch with no major pixel differences. If this was an issue wouldn't painting left to right cause issues with content overflowing to the right?
Julien Chaffraix
Comment 25
2011-12-05 16:29:43 PST
(In reply to
comment #24
)
> (In reply to
comment #23
) > > (From update of
attachment 117792
[details]
[details]) > > I am pretty sure this will make us paint badly some cases. Think of cases involving a cell with some overflow on the left of it: with your patch, the painting order will be inverted (the overflow will be under the left box's content). You can add outline, box-shadow, negative margins, negative top/left to this example. > > tables/mozilla_expected_failures/marvin/table_overflow_* seem to cover this kind of thing. One of them is rebaselined in this patch with no major pixel differences.
Actually if you pay attention, your rebaselined change is the difference I mention. It is subtle (1px difference) and it took me some time to see it: the difference is at the intersection between the red border and the border of the left cell. The red border used to be on top but your change made the left cell's border to be on top. If you want to make the change more explicit, add a background color to the left cell and the overflow.
> If this was an issue wouldn't painting left to right cause issues with content overflowing to the right?
No, AFAICT you are expected to paint in tree order in tables ("left-to-right" is dangerous to use with writing modes and that's my mistake to have used "left" in the first place) so the situation is not symmetric.
Robert Hogan
Comment 26
2011-12-07 13:30:43 PST
Created
attachment 118262
[details]
Patch
Robert Hogan
Comment 27
2011-12-07 13:35:38 PST
(In reply to
comment #25
)
> > Actually if you pay attention, your rebaselined change is the difference I mention. It is subtle (1px difference) and it took me some time to see it: the difference is at the intersection between the red border and the border of the left cell. The red border used to be on top but your change made the left cell's border to be on top. If you want to make the change more explicit, add a background color to the left cell and the overflow.
My solution to this is to paint collapsed borders separately from cells in RenderTableSection. This doesn't introduce any performance overhead as that is what is happening effectively anyway. This approach produces no suspicious-looking pixel differences so hopefully is acceptable!
Darin Adler
Comment 28
2011-12-07 14:24:20 PST
Comment on
attachment 118262
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=118262&action=review
> Source/WebCore/rendering/RenderTableCell.cpp:807 > void RenderTableCell::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
Should just remove this rather than having it call through to the base class.
> Source/WebCore/rendering/style/CollapsedBorderValue.h:56 > bool operator==(const CollapsedBorderValue& o) const > { > - return m_border == o.m_border && m_borderColor == o.m_borderColor && m_precedence == o.m_precedence; > + return m_border.width() == o.m_border.width() && m_border.style() == o.m_border.style() && m_precedence == o.m_precedence; > }
Having this equality operation is fine, but it should not be named "==". It should be a named function instead if it's not a real equality comparison.
Julien Chaffraix
Comment 29
2011-12-07 16:28:42 PST
Comment on
attachment 118262
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=118262&action=review
I think this approach could work. Some comments on the implementation though.
>> Source/WebCore/rendering/RenderTableCell.cpp:807
> > Should just remove this rather than having it call through to the base class.
I am ambivalent about that: I would like the following ASSERT added either here or in RenderBlock::paint. ASSERT(!(paintInfo.phase == PaintPhaseCollapsedTableBorders)); In RenderBlock it would catch more abuse as this phase should be limited to the table code. However it also exposing this phase in RenderBlock that should not know about it.
> Source/WebCore/rendering/RenderTableCell.cpp:813 > +{
Can you add the following ASSERT: ASSERT(paintInfo.phase == PaintPhaseCollapsedTableBorders);
> Source/WebCore/rendering/RenderTableCell.cpp:814 > + if (!paintInfo.shouldPaintWithinRoot(this))
The style()->visibility() == VISIBLE check should be kept here (see comments below).
> Source/WebCore/rendering/RenderTableCell.cpp:822 > + return;
unneeded return.
> Source/WebCore/rendering/RenderTableCell.h:125 > + void paintCellCollapsedBorders(PaintInfo&, const LayoutPoint&);
Do we really need a separate function or could we inline this shim function in paintCollapsedBorder? Also paintCollapsedBorders would make a lot more sense here.
> Source/WebCore/rendering/RenderTableSection.cpp:1063 > + if (paintInfo.phase == PaintPhaseCollapsedTableBorders && style()->visibility() == VISIBLE) {
This check does not replace a check for each cell. It would match: <table><thead style="visibility: hidden"><td></td></thead></table> but would miss: <table><thead><td style="visibility: hidden"></td></thead></table> This is also not the right place to put this check (same comment for the other check). It should be added at the beginning of the function to avoid all the computing we are doing above. This is a separate change, could it be postponed to another bug please?
> Source/WebCore/rendering/RenderTableSection.cpp:1080 > + for (unsigned r = startrow; r < endrow; r++) {
r -> row for consistency.
> Source/WebCore/rendering/RenderTableSection.cpp:1081 > + for (unsigned c = startcol; c < endcol; c++) {
c -> col for consistency.
> Source/WebCore/rendering/RenderTableSection.cpp:1086 > + paintCell(cell, paintInfo, paintOffset);
There is a check for paintInfo.phase == PaintPhaseCollapsedTableBorders in paintCell which should be removed as it will never be hit.
> Source/WebCore/rendering/RenderTableSection.cpp:1128 > + LayoutPoint cellPoint = flipForWritingModeForChild(cells[i-1], paintOffset);
That's cool that you fix the code for writing mode! Are different writing mode covered in the tests you check in?
> Source/WebCore/rendering/RenderTableSection.cpp:1129 > + cells[i-1]->paintCellCollapsedBorders(paintInfo, cellPoint);
Style: cells[i - 1] (twice in the 2 previous lines).
Robert Hogan
Comment 30
2011-12-09 12:22:22 PST
Created
attachment 118609
[details]
Patch
Darin Adler
Comment 31
2011-12-09 14:12:37 PST
Comment on
attachment 118609
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=118609&action=review
> Source/WebCore/rendering/RenderTableCell.cpp:809 > + ASSERT(!(paintInfo.phase == PaintPhaseCollapsedTableBorders));
Probably would read better with !=.
> Source/WebCore/rendering/RenderTableCell.cpp:896 > + if ((*a).isSameIgnoringColor(*b))
Should use the -> operator here instead of (*a).
> Source/WebCore/rendering/RenderTableSection.cpp:998 > + if ((!cell->hasSelfPaintingLayer() && !row->hasSelfPaintingLayer()))
There’s an extra set of parentheses here we should remove.
Julien Chaffraix
Comment 32
2011-12-09 14:59:20 PST
Comment on
attachment 118609
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=118609&action=review
Yay to ref-tests!
> Source/WebCore/rendering/RenderTableSection.cpp:1063 > + if (paintInfo.phase == PaintPhaseCollapsedTableBorders && style()->visibility() == VISIBLE) {
the style()->visibility() == VISIBLE is a good optimization but it is not related to this bug and I am pretty sure we are still doing too much work in this case. Could this be pushed to another bug?
> Source/WebCore/rendering/RenderTableSection.cpp:1126 > + if (paintInfo.phase == PaintPhaseCollapsedTableBorders && style()->visibility() == VISIBLE) {
Ditto.
Robert Hogan
Comment 33
2011-12-19 11:50:26 PST
Committed
r103251
: <
http://trac.webkit.org/changeset/103251
>
Ryosuke Niwa
Comment 34
2012-01-10 01:57:02 PST
A whole bunch of layout tests have been failing on Apple's Mac port since this patch was landed:
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r104531%20(36197)/results.html
Could you either rebaseline these tests or fix whatever regressions this patch may have caused?
Robert Hogan
Comment 35
2012-01-10 10:29:28 PST
(In reply to
comment #34
)
> A whole bunch of layout tests have been failing on Apple's Mac port since this patch was landed: > >
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r104531%20(36197)/results.html
> > Could you either rebaseline these tests or fix whatever regressions this patch may have caused?
The expectations need to be fixed to prevent them displaying on the results page. They're suppressed in the expectations file, see the items listed for BUGWK74888 in:
http://trac.webkit.org/browser/trunk/LayoutTests/platform/mac/test_expectations.txt
Unfortunately, the SL bot doesn't test png results, so the failure is TEXT and not IMAGE+TEXT. That's the reason they show up in the results page. They aren't counted as failures as you can see from:
http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20%28Tests%29/builds/36197/steps/layout-test/logs/stdio
Those failing canvas tests aren't related to the patch here. All of which is just FYI, I need to fix the expectations to TEXT for Mac until someone in Apple has the time to rebaseline. Though if there is a Mac buildbot that checks png results then *it* will start getting this cruft in its results page.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug