Bug 98847 – [Chromium] Rebaseline hyphenation tests

• Component: New Bugs
• Status: RESOLVED WONTFIX

[Hironori Bono]

[Chromium] Rebaseline hyphenation tests

[Hironori Bono]

Created attachment 167908 [details]
Patch

[Hironori Bono]

Greetings,

Even though a few hyphenation tests still fail due to incompatibility with Core Foundation, I would like to update text expectations for hyphenation tests that look OK. (These tests pass on Mac without rebaselines.) Would it be possible to review this change?

Regards,

Hironori Bono

[WebKit Review Bot]

Comment on attachment 167908 [details]
Patch

Attachment 167908 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14229907

New failing tests:
fast/text/hyphenate-first-word.html
fast/text/hyphenate-character.html
fast/text/hyphens.html
fast/text/hyphen-min-preferred-width.html

[Tony Chang]

Comment on attachment 167908 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=167908&action=review

I'm not sure why the tests failed on Linux.  Did you use garden-o-matic to get results from the bots?

> LayoutTests/ChangeLog:8
> +        This change adds rebaselined results for hyphnation tests and updates their

typo: hyphnation -> hyphenation

[Tony Chang]

Comment on attachment 167908 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=167908&action=review

> LayoutTests/platform/chromium/TestExpectations:2251
> +# Fails due to incompatibility with Core Foundation. These tests should be changed to ref tests to conceal it.
>  crbug.com/47083 fast/text/hyphenate-locale.html [ Failure ]

Do you mean they insert a hyphen at a different location?  If so, I would just go ahead and re-baseline on Mac as well.  I think it's OK that we put the hyphens in different locations.

[Hironori Bono]

Greetings Tony,

Thank you for your review and a comment. In brief, Chromium DumpRenderTree does not insert hyphens to the locations expected by these tests. (These tests heavily depend on the fonts, including subsampling, used by Apple DumpRenderTree.) Even though we can rebaseline these tests, I would like to change these tests themselves to avoid rebaselines in the future. (We can easily create expected results for these tests with soft hyphens '­'.)
By the way, I wonder why this change fail on EWS bots because I have created this change with garden-o-matic. (Someone changed rendering code after I ran garden-o-matic?)

Regards,

Hironori Bono

(In reply to comment #5)
> Do you mean they insert a hyphen at a different location?  If so, I would just go ahead and re-baseline on Mac as well.  I think it's OK that we put the hyphens in different locations.

[Tony Chang]

(In reply to comment #6)
> Thank you for your review and a comment. In brief, Chromium DumpRenderTree does not insert hyphens to the locations expected by these tests. (These tests heavily depend on the fonts, including subsampling, used by Apple DumpRenderTree.) Even though we can rebaseline these tests, I would like to change these tests themselves to avoid rebaselines in the future. (We can easily create expected results for these tests with soft hyphens '­'.)

Right, different text width will cause the text to wrap and break in different places.  If you can come up with a way to make it consistent across platforms as a reftest, that would be great!  I think it might be hard to get the spacing consistent across platforms, but maybe you can use the ahem font.

> By the way, I wonder why this change fail on EWS bots because I have created this change with garden-o-matic. (Someone changed rendering code after I ran garden-o-matic?)

Hmm, I'm not sure either.  Maybe try using garden-o-matic again and uploading it again and seeing if it fails a second time?

[Hironori Bono]

Created attachment 168377 [details]
Patch

[Hironori Bono]

Greetings Tony,

Thank you for your review and comments. Even though I have re-written hyphneation tests with ref tests, I notice the current DumpRenderTree of Chromium has problems and a couple of tests still fail. I have written a Chromium change <http://codereview.chromium.org/11112011/> that fixes these issues. (I have set r- for my updated patch because it fails on EWS bots.)

Regards,

Hironori Bono

(In reply to comment #7)
> Right, different text width will cause the text to wrap and break in different places.  If you can come up with a way to make it consistent across platforms as a reftest, that would be great!  I think it might be hard to get the spacing consistent across platforms, but maybe you can use the ahem font.

[Tony Chang]

This approach looks good to me as long as the ref tests pass on Apple Mac as well.