Bug 104160

Created attachment 177832 [details] Patch

Comment on attachment 177832 [details] Patch Clearing flags on attachment: 177832 Committed r136835: <http://trac.webkit.org/changeset/136835>

All reviewed patches have been landed. Closing bug.

Hi Yael, The purpose of the test was to check that max-width is honored in the case of a region with width: auto. The text, both in original and expected, is not supposed to fit in 400px, but rather to overflow the region and the div containing the test in expected file. In both cases, it should have shown about 10 lines of gray text (last one shorter) because neither the region nor the div containing the text have overflow:hidden. Because of that, changing the height to 450 seems like a wrong way to fix this test. I have build QT on Linux with latest and also revision 136322 from Dec 2 and i have changed the height to 400, without noticing any diff. Would you mind telling me what revision on QT have you used to notice the difference? Cheers, Mihnea

(In reply to comment #4) > Hi Yael, > > The purpose of the test was to check that max-width is honored in the case of a region with width: auto. The text, both in original and expected, is not supposed to fit in 400px, but rather to overflow the region and the div containing the test in expected file. In both cases, it should have shown about 10 lines of gray text (last one shorter) because neither the region nor the div containing the text have overflow:hidden. > > Because of that, changing the height to 450 seems like a wrong way to fix this test. I have build QT on Linux with latest and also revision 136322 from Dec 2 and i have changed the height to 400, without noticing any diff. Would you mind telling me what revision on QT have you used to notice the difference? > > Cheers, > Mihnea Hi Mihnea, Sorry for misunderstanding the test and also for cc'ing the wrong person on this bug. If you think that the original version is the correct version of the test, please feel free to roll out. I just tested with r137836 on Qt's MiniBrowser and if I change the 450 back to 400, in fast/regions/autowidth-normalflow-maxwidth.html, the test is failing. With 450 it does not fail.

(In reply to comment #5) > (In reply to comment #4) > > Hi Yael, > > > > The purpose of the test was to check that max-width is honored in the case of a region with width: auto. The text, both in original and expected, is not supposed to fit in 400px, but rather to overflow the region and the div containing the test in expected file. In both cases, it should have shown about 10 lines of gray text (last one shorter) because neither the region nor the div containing the text have overflow:hidden. > > > > Because of that, changing the height to 450 seems like a wrong way to fix this test. I have build QT on Linux with latest and also revision 136322 from Dec 2 and i have changed the height to 400, without noticing any diff. Would you mind telling me what revision on QT have you used to notice the difference? > > > > Cheers, > > Mihnea > > > Hi Mihnea, Sorry for misunderstanding the test and also for cc'ing the wrong person on this bug. If you think that the original version is the correct version of the test, please feel free to roll out. > > I just tested with r137836 on Qt's MiniBrowser and if I change the 450 back to 400, in fast/regions/autowidth-normalflow-maxwidth.html, the test is failing. With 450 it does not fail. Ok, now i see the failure and i need to investigate it. I was not testing with WK2.