This is a reduction of an issue seen at build.webkit.org/console The columns with 'texttesttest' collapse to a tiny width if the colspan is set to 3, but not when set to 2 or 4.
Created attachment 119891 [details] Reduction - colspan 2 looks fine
Created attachment 119892 [details] Reduction - colspan 3 fails
Created attachment 122687 [details] Patch
Comment on attachment 122687 [details] Patch Attachment 122687 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11268065 New failing tests: fast/css/min-width-with-spanned-cell.html
Created attachment 122989 [details] Patch
Comment on attachment 122989 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=122989&action=review Good catch, the code part looks fine. > LayoutTests/ChangeLog:17 > + * platform/chromium-linux/fast/table/027-expected.png: > + * platform/chromium-linux/fast/table/027-vertical-expected.png: > + * platform/chromium-win/fast/table/027-expected.txt: > + * platform/chromium-win/fast/table/027-vertical-expected.txt: Those 2 tests should be disabled on all other platforms to avoid breaking the tree. > LayoutTests/fast/css/min-width-with-spanned-cell-fixed.html:15 > +<!--When a cell spans an entire row on a table with fixed layout, allow cells containing text to be > +squeezed smaller than the width of the text.--> The comment is wrong, it says the opposite of the other test. It would be nice to add what you expect to see in the pixel dump too for both tests. > LayoutTests/platform/chromium-win/fast/table/027-vertical-expected.txt:40 > - RenderTableRow {TR} at (0,28) size 344x744 > + RenderTableRow {TR} at (0,28) size 344x756 I am concerned about this change. Why is it changing so much when it was a 1px change in an horizontal-writing mode? Have you checked if our new size matches more closely FF / IE now?
*** Bug 76807 has been marked as a duplicate of this bug. ***
Comment on attachment 122989 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=122989&action=review >> LayoutTests/fast/css/min-width-with-spanned-cell-fixed.html:15 >> +squeezed smaller than the width of the text.--> > > The comment is wrong, it says the opposite of the other test. > > It would be nice to add what you expect to see in the pixel dump too for both tests. The text is intentional. This is the way it works for fixed width tables afaict. >> LayoutTests/platform/chromium-win/fast/table/027-vertical-expected.txt:40 >> + RenderTableRow {TR} at (0,28) size 344x756 > > I am concerned about this change. Why is it changing so much when it was a 1px change in an horizontal-writing mode? > > Have you checked if our new size matches more closely FF / IE now? It's a progression - the image in the last cell no longer extrudes from the table when the height of the view is lower thin the vertical 'width' of the table.
Created attachment 123590 [details] Patch
Comment on attachment 123590 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123590&action=review The patch does not apply on ToT because your win expectation file change needs to be rebased. > LayoutTests/ChangeLog:18 > + * platform/chromium-linux/fast/table/027-expected.png: > + 1 px difference - benign > + * platform/chromium-linux/fast/table/027-vertical-expected.png: > + This is a progression - previously the image in the last cell was > + extruding from the table in a 800x600 view. Nit: Spaces would make those 2 comments stand out more. > LayoutTests/platform/mac/test_expectations.txt:201 > +BUGWK74874 : fast/table/027.html = TEXT > +BUGWK74874 : fast/table/027-vertical.html = TEXT Unless I am missing something, this should also be TEXT+IMAGE as you haven't rebaselined Mac in this patch.
(In reply to comment #10) > > > LayoutTests/platform/mac/test_expectations.txt:201 > > +BUGWK74874 : fast/table/027.html = TEXT > > +BUGWK74874 : fast/table/027-vertical.html = TEXT > > Unless I am missing something, this should also be TEXT+IMAGE as you haven't rebaselined Mac in this patch. The mac bots don't run pixel tests - so will only ever see the TEXT difference.
Committed r105775: <http://trac.webkit.org/changeset/105775>
(In reply to comment #11) > (In reply to comment #10) > > > > > LayoutTests/platform/mac/test_expectations.txt:201 > > > +BUGWK74874 : fast/table/027.html = TEXT > > > +BUGWK74874 : fast/table/027-vertical.html = TEXT > > > > Unless I am missing something, this should also be TEXT+IMAGE as you haven't rebaselined Mac in this patch. > > The mac bots don't run pixel tests - so will only ever see the TEXT difference. That is not a good reason not to put TEXT+IMAGE. I am sure some developers - apart from myself - are running the tests on Mac with pixel dumps enabled and they will see an unexpected image difference. On top of that, the test_expectations.txt files are annotation on the state of our tests. Here you are doing only half of the annotation for the future maintainer looking at the failure.
(In reply to comment #13) > (In reply to comment #11) > > (In reply to comment #10) > > > > > > > LayoutTests/platform/mac/test_expectations.txt:201 > > > > +BUGWK74874 : fast/table/027.html = TEXT > > > > +BUGWK74874 : fast/table/027-vertical.html = TEXT > > > > > > Unless I am missing something, this should also be TEXT+IMAGE as you haven't rebaselined Mac in this patch. > > > > The mac bots don't run pixel tests - so will only ever see the TEXT difference. > > That is not a good reason not to put TEXT+IMAGE. I am sure some developers - apart from myself - are running the tests on Mac with pixel dumps enabled and they will see an unexpected image difference. On top of that, the test_expectations.txt files are annotation on the state of our tests. Here you are doing only half of the annotation for the future maintainer looking at the failure. Right, but using TEXT+IMAGE means that the bots will report them as unexpected failures in the results.html - even though they're not listed as regressions in the results stdio. See https://bugs.webkit.org/show_bug.cgi?id=71244#c35 where this came up previously. The lack of a Mac gardener really hurts in this situation.
Landed results for Qt and Gtk: http://trac.webkit.org/changeset/105799
> > That is not a good reason not to put TEXT+IMAGE. I am sure some developers - apart from myself - are running the tests on Mac with pixel dumps enabled and they will see an unexpected image difference. On top of that, the test_expectations.txt files are annotation on the state of our tests. Here you are doing only half of the annotation for the future maintainer looking at the failure. > > Right, but using TEXT+IMAGE means that the bots will report them as unexpected failures in the results.html - even though they're not listed as regressions in the results stdio. > > See https://bugs.webkit.org/show_bug.cgi?id=71244#c35 where this came up previously. The lack of a Mac gardener really hurts in this situation. Ah, that my friend is a good explanation of why we don't want the IMAGE part. I agree that it is unfortunate that we have to do that just because of snow-leopard.
<rdar://problem/10960651>