Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(171)

Issue 1203743002: Include glyph overflows caused by negative spacing into visual overflow (Closed)

Created:
4 years, 10 months ago by Xianzhu
Modified:
4 years, 10 months ago
Reviewers:
chrishtr, eae
CC:
blink-reviews, blink-reviews-rendering, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, szager+layoutwatch_chromium.org, zoltan1
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Include glyph overflows caused by negative spacing into visual overflow BTW Modified the test to reproduce the bug without strict culling. BUG=496317 TEST=fast/text/international/rtl-negative-letter-spacing.html Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197684

Patch Set 1 #

Patch Set 2 : #

Total comments: 2

Patch Set 3 : Add comments about negative word-spacing/letter-spacing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -7 lines) Patch
M LayoutTests/TestExpectations View 1 1 chunk +3 lines, -0 lines 0 comments Download
M LayoutTests/fast/text/international/rtl-negative-letter-spacing.html View 1 chunk +4 lines, -3 lines 0 comments Download
M Source/core/layout/LayoutBlockFlowLine.cpp View 1 2 2 chunks +12 lines, -4 lines 0 comments Download

Messages

Total messages: 11 (3 generated)
Xianzhu
4 years, 10 months ago (2015-06-23 20:36:58 UTC) #2
Xianzhu
4 years, 10 months ago (2015-06-23 20:36:59 UTC) #3
Xianzhu
Ptal.
4 years, 10 months ago (2015-06-23 22:24:52 UTC) #4
eae
LGTM w/nit https://codereview.chromium.org/1203743002/diff/20001/Source/core/layout/LayoutBlockFlowLine.cpp File Source/core/layout/LayoutBlockFlowLine.cpp (right): https://codereview.chromium.org/1203743002/diff/20001/Source/core/layout/LayoutBlockFlowLine.cpp#newcode435 Source/core/layout/LayoutBlockFlowLine.cpp:435: if (measuredWidth < 0) { This could ...
4 years, 10 months ago (2015-06-23 22:28:54 UTC) #5
chrishtr
I'll defer to Emil's review, he is more versed in this code.
4 years, 10 months ago (2015-06-23 22:31:04 UTC) #6
Xianzhu
https://codereview.chromium.org/1203743002/diff/20001/Source/core/layout/LayoutBlockFlowLine.cpp File Source/core/layout/LayoutBlockFlowLine.cpp (right): https://codereview.chromium.org/1203743002/diff/20001/Source/core/layout/LayoutBlockFlowLine.cpp#newcode435 Source/core/layout/LayoutBlockFlowLine.cpp:435: if (measuredWidth < 0) { On 2015/06/23 22:28:54, eae ...
4 years, 10 months ago (2015-06-23 22:43:33 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1203743002/40001
4 years, 10 months ago (2015-06-23 22:43:55 UTC) #10
commit-bot: I haz the power
4 years, 10 months ago (2015-06-24 00:05:02 UTC) #11
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197684

Powered by Google App Engine
This is Rietveld 408576698