Bug 151051 – tate-chu-yoko should shrink to fit when it exceeds the available space

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Dave Hyatt]

tate-chu-yoko needs to shrink to fit (within reason) if it exceeds the amount of available space.

[Dave Hyatt]

Created attachment 265089 [details]
Patch

[Dave Hyatt]

Created attachment 265092 [details]
Patch

[Myles C. Maxfield]

Comment on attachment 265092 [details]
Patch

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

It seems to me that this is going to squish the tatechuyoko vertically as well as horizontally. Are we sure that fits the bill? I expected you to do something with matrix transformations to squish in only a single direction.

> Source/WebCore/rendering/RenderCombineText.cpp:121
> +    

Please remove this whitespace.

> Source/WebCore/rendering/RenderCombineText.cpp:133
> +            glyphOverflow.left = glyphOverflow.top = glyphOverflow.right = glyphOverflow.bottom = 0;

Is this necessary? Shouldn't the callee be doing this?

> Source/WebCore/rendering/RenderCombineText.cpp:159
> +            FontCascade compressedFont(bestFitDescription, style().fontCascade().letterSpacing(), style().fontCascade().wordSpacing());

Shouldn't we be picking between style() and firstLineStyle() appropriately?

> Source/WebCore/rendering/RenderCombineText.cpp:169
> +            computedSize--;

I don't think we should be looping on the order of the original font size. Isn't there a way to take bigger steps the further out we are?

[Myles C. Maxfield]

Comment on attachment 265092 [details]
Patch

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

> LayoutTests/fast/text/text-combine-crash-expected.txt:-5
> -foo   

What? Why did this change?

> LayoutTests/fast/text/international/spaces-combined-in-vertical-text-expected.txt:-1
> -PASS if no ASSERT fail or crash in debug build.

What? Why did this change?

[Build Bot]

Comment on attachment 265092 [details]
Patch

Attachment 265092 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/407053

New failing tests:
fast/text/international/text-combine-image-test.html

[Build Bot]

Created attachment 265095 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Myles C. Maxfield]

Comment on attachment 265092 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Covered by existing tests

We should make sure that RenderCombineText::width() uses the correct (shrunken) font too (it looks like it does from the .pngs but it isn't obvious)

> LayoutTests/platform/mac/fast/text/decorations-with-text-combine-expected.txt:-40
> -              text run at (6,288) width 71: "12345"

I'm not sure if combining 12345 here is best. But I'm also not sure if keeping it uncombined here is best. Hrm.

[Myles C. Maxfield]

Comment on attachment 265092 [details]
Patch

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

> Source/WebCore/rendering/RenderCombineText.cpp:170
> +        } while (computedSize >= 6.0f);

Hardcoding this 6 here is probably bad, given the original font size could be humongous, making 6px way way too small relatively.

[Myles C. Maxfield]

Comment on attachment 265092 [details]
Patch

After some research, it appears that shrinking in both dimensions is fine.

[Build Bot]

Comment on attachment 265092 [details]
Patch

Attachment 265092 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/407081

New failing tests:
fast/text/international/text-combine-image-test.html

[Build Bot]

Created attachment 265096 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Dave Hyatt]

Created attachment 265205 [details]
Patch for EWS testing

[Build Bot]

Comment on attachment 265205 [details]
Patch for EWS testing

Attachment 265205 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/411302

New failing tests:
fast/text/international/text-combine-image-test.html

[Build Bot]

Created attachment 265213 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 265205 [details]
Patch for EWS testing

Attachment 265205 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/411353

New failing tests:
fast/text/international/text-combine-image-test.html

[Build Bot]

Created attachment 265217 [details]
Archive of layout-test-results from ews100 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Dave Hyatt]

Fixed in r192269.

[Koji Ishii]

*** Bug 93832 has been marked as a duplicate of this bug. ***

[Csaba Osztrogonác]

(In reply to comment #17)
> Fixed in r192269.

The commit log was repeated 47 times.  :(  Dave, could you check your script?