|Summary:||Convert some fast/regions pixel tests to reftests|
|Product:||WebKit||Reporter:||Jacob Goldstein <jacobg>|
|Component:||Tools / Tests||Assignee:||Jacob Goldstein <jacobg>|
|Severity:||Normal||CC:||dglazkov, jacobg, tony, webkit.review.bot|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.6|
Description Jacob Goldstein 2011-12-08 15:07:51 PST
Convert some fast/regions pixel tests to reftests
Comment 6 WebKit Review Bot 2011-12-14 12:42:09 PST
Comment on attachment 119267 [details] Patch Attachment 119267 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10873348 New failing tests: fast/regions/webkit-flow-inlines-dynamic.html
Comment 7 Jacob Goldstein 2011-12-15 10:21:51 PST
Failure listed in Comment #6 not associated with the test changed by this patch
Comment 8 Jacob Goldstein 2011-12-15 10:29:18 PST
Failure is related - investigating (In reply to comment #7) > Failure listed in Comment #6 not associated with the test changed by this patch
Comment 9 Tony Chang 2011-12-15 13:34:54 PST
I suspect the failure is because on different OSes, the word wrapping will happen at different places. This is probably not the easiest test to convert. You could maybe change the text to spans with a fixed width or try using the Ahem font.
Comment 11 WebKit Review Bot 2011-12-15 16:06:42 PST
Comment on attachment 119494 [details] Patch Attachment 119494 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10914419 New failing tests: fast/regions/webkit-flow-inlines-dynamic.html
Comment 12 Jacob Goldstein 2011-12-16 11:07:34 PST
Created attachment 119635 [details] Patch Removed change to test_expectations.txt - results from the updated test will still be ignored on Chromium
Comment 13 Tony Chang 2011-12-16 11:12:26 PST
Comment on attachment 119635 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119635&action=review > LayoutTests/ChangeLog:16 > +2011-12-15 Eric Carlson <firstname.lastname@example.org> > + Looks like the ChangeLog files didn't merge properly.
Comment 14 Jacob Goldstein 2011-12-16 11:29:43 PST
Created attachment 119640 [details] Patch Corrected ChangeLog
Comment 15 Tony Chang 2011-12-16 11:42:55 PST
Comment on attachment 119640 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119640&action=review > LayoutTests/ChangeLog:4 > + updates > + Did you mean to include these lines? Maybe just remove them. > LayoutTests/fast/regions/webkit-flow-inlines-dynamic.html:39 > > <div id="content"> Could you add some text saying what the test is testing? E.g., The text in the test should not get out of the region. > LayoutTests/fast/regions/webkit-flow-inlines-dynamic.html:-58 > -<script> > -document.body.offsetWidth; > -document.getElementById('test').appendChild(document.createTextNode(" Here is some added content that will mess things up, since it shoves things down.")); Is this an important part of the test? Why is it being removed?
Comment 19 Jacob Goldstein 2012-01-16 14:25:58 PST
Latest patch address Comment #15: - Updated ChangeLog to remove addition line - Added text to explain what is being tested - Modified test to include dynamic insertion of text to verify that existing text remains within region after insertion NOTE: The text being used in the test files was changed to help protect against failures on various platforms where font size may result in slightly difference spacing. With the new text being used, slightly changes should not affect text wrapping
Comment 20 Tony Chang 2012-01-17 11:46:42 PST
Comment on attachment 122684 [details] Patch Can we use the ahem font to make the test even more cross platform compatible?
Comment 21 Jacob Goldstein 2012-01-17 12:46:09 PST
Created attachment 122800 [details] Patch Updated to use the Ahem font
Comment 22 WebKit Review Bot 2012-01-17 15:26:08 PST
Comment on attachment 122800 [details] Patch Clearing flags on attachment: 122800 Committed r105200: <http://trac.webkit.org/changeset/105200>
Comment 23 WebKit Review Bot 2012-01-17 15:26:15 PST
All reviewed patches have been landed. Closing bug.