Bug 151051

Summary:

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

Product:

WebKit

Reporter:

Dave Hyatt <hyatt>

Component:

Layout and Rendering

Assignee:

Dave Hyatt <hyatt>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, commit-queue, esprehn+autocc, glenn, kojii, kondapallykalyan, mmaxfield, ossy, rniwa

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	mmaxfield: review+, buildbot: commit-queue-
Archive of layout-test-results from ews102 for mac-mavericks	none
Archive of layout-test-results from ews105 for mac-mavericks-wk2	none
Patch for EWS testing	buildbot: commit-queue-
Archive of layout-test-results from ews105 for mac-mavericks-wk2	none
Archive of layout-test-results from ews100 for mac-mavericks	none

Dave Hyatt

Reported 2015-11-09 13:10:01 PST

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

Attachments
Patch (141.78 KB, patch) 2015-11-09 13:14 PST, Dave Hyatt	no flags	Details Formatted Diff Diff
Patch (136.30 KB, patch) 2015-11-09 13:26 PST, Dave Hyatt	mmaxfield: review+ buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews102 for mac-mavericks (1.12 MB, application/zip) 2015-11-09 13:57 PST, Build Bot	no flags	Details
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (1.09 MB, application/zip) 2015-11-09 14:08 PST, Build Bot	no flags	Details
Patch for EWS testing (135.89 KB, patch) 2015-11-10 11:33 PST, Dave Hyatt	buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (1.04 MB, application/zip) 2015-11-10 12:08 PST, Build Bot	no flags	Details
Archive of layout-test-results from ews100 for mac-mavericks (878.11 KB, application/zip) 2015-11-10 12:19 PST, Build Bot	no flags	Details
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Dave Hyatt

Comment 1 2015-11-09 13:14:37 PST

Created attachment 265089 [details] Patch

Dave Hyatt

Comment 2 2015-11-09 13:26:11 PST

Created attachment 265092 [details] Patch

Myles C. Maxfield

Comment 3 2015-11-09 13:50:24 PST

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 4 2015-11-09 13:53:44 PST

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 5 2015-11-09 13:57:41 PST

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

Comment 6 2015-11-09 13:57:45 PST

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 7 2015-11-09 14:00:10 PST

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 8 2015-11-09 14:01:55 PST

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 9 2015-11-09 14:08:21 PST

Comment on attachment 265092 [details] Patch After some research, it appears that shrinking in both dimensions is fine.

Build Bot

Comment 10 2015-11-09 14:08:30 PST

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

Comment 11 2015-11-09 14:08:35 PST

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

Comment 12 2015-11-10 11:33:58 PST

Created attachment 265205 [details] Patch for EWS testing

Build Bot

Comment 13 2015-11-10 12:08:47 PST

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

Comment 14 2015-11-10 12:08:51 PST

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 15 2015-11-10 12:19:06 PST

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

Comment 16 2015-11-10 12:19:10 PST

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

Comment 17 2015-11-10 14:05:12 PST

Fixed in r192269.

Koji Ishii

Comment 18 2015-11-20 00:00:35 PST

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

Csaba Osztrogonác

Comment 19 2016-01-05 00:56:52 PST

(In reply to comment #17) > Fixed in r192269. The commit log was repeated 47 times. :( Dave, could you check your script?

Note You need to log in before you can comment on or make changes to this bug.