Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(276)

Issue 529753002: Word-Spacing in InlineBox RTL scenario not to ignore if first character is space (Closed)

Created:
6 years, 3 months ago by Habib Virji
Modified:
6 years, 3 months ago
CC:
blink-reviews, blink-reviews-rendering, zoltan1, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, pdr., rune+blink
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Word-Spacing in InlineBox RTL scenario not to ignore if first character is space InlineFlowBox, sets word spacing first if in LTR scenario. In RTL scenario first logicalLeft should be set prior to setting word-spacing length. RenderBlockLineLayout, sets needsWordSpacing as true to not ignore space if first character. BUG=344873 R=behdad,eae, aharon TEST=fast/text/international/bidi-word-spacing.html Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181636

Patch Set 1 #

Patch Set 2 : Fix tests issue for css/word-space-extra #

Patch Set 3 : "Updated to latest master" #

Patch Set 4 : TestExpectations update #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -5 lines) Patch
M LayoutTests/TestExpectations View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M LayoutTests/platform/linux/fast/text/international/bidi-word-spacing-rtl-expected.png View Binary file 0 comments Download
M LayoutTests/platform/linux/fast/text/international/bidi-word-spacing-rtl-expected.txt View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/rendering/InlineFlowBox.cpp View 1 chunk +9 lines, -2 lines 0 comments Download
M Source/core/rendering/RenderBlockLineLayout.cpp View 1 2 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 8 (3 generated)
Habib Virji
6 years, 3 months ago (2014-09-01 14:09:46 UTC) #1
Habib Virji
On 2014/09/01 14:09:46, Habib Virji wrote: @eae can you please have a look.
6 years, 3 months ago (2014-09-08 18:39:50 UTC) #4
eae
LGTM, Thank you.
6 years, 3 months ago (2014-09-08 19:00:01 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/habib.virji@samsung.com/529753002/60001
6 years, 3 months ago (2014-09-09 09:54:41 UTC) #7
commit-bot: I haz the power
6 years, 3 months ago (2014-09-09 11:31:46 UTC) #8
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as 181636

Powered by Google App Engine
This is Rietveld 408576698