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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 23298002: Fix nested unicode-bidi: isolate (Closed)

Created:
7 years, 4 months ago by igoroliveira
Modified:
7 years, 4 months ago
CC:
blink-reviews, dglazkov+blink, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, jww
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Fix nested unicode-bidi: isolate When we have a nested isolate renderer if the outer isolate has a text and this text is not the first child, the isolated chain is not rendered correctly. This happens because constructBidiRunsForSegment uses always the first inner isolated renderer as isolated inline container. This patch fixes the behavior described changing containingIsolate to find the right isolated container. BUG=274717 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=156261

Patch Set 1 #

Total comments: 1

Patch Set 2 : Proposed patch #

Messages

Total messages: 9 (0 generated)
igoroliveira
7 years, 4 months ago (2013-08-16 21:22:11 UTC) #1
leviw_travelin_and_unemployed
https://codereview.chromium.org/23298002/diff/1/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-simple.html File LayoutTests/fast/text/international/unicode-bidi-isolate-nested-simple.html (right): https://codereview.chromium.org/23298002/diff/1/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-simple.html#newcode1 LayoutTests/fast/text/international/unicode-bidi-isolate-nested-simple.html:1: <!DOCTYPE html> How about making these Ref tests? I ...
7 years, 4 months ago (2013-08-16 21:32:11 UTC) #2
igoroliveira
On 2013/08/16 21:32:11, Levi wrote: > https://codereview.chromium.org/23298002/diff/1/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-simple.html > File LayoutTests/fast/text/international/unicode-bidi-isolate-nested-simple.html > (right): > > https://codereview.chromium.org/23298002/diff/1/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-simple.html#newcode1 ...
7 years, 4 months ago (2013-08-16 21:47:24 UTC) #3
eseidel
May relate to Joel's current bug. :) Looks fine by me. I'll let leviw do ...
7 years, 4 months ago (2013-08-16 21:52:22 UTC) #4
leviw_travelin_and_unemployed
LGTM. Thanks for fixing up the tests :)
7 years, 4 months ago (2013-08-16 21:54:49 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/igor.o@sisa.samsung.com/23298002/6001
7 years, 4 months ago (2013-08-16 21:59:33 UTC) #6
commit-bot: I haz the power
Retried try job too often on win_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_rel&number=2536
7 years, 4 months ago (2013-08-17 00:41:30 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/igor.o@sisa.samsung.com/23298002/6001
7 years, 4 months ago (2013-08-17 04:04:29 UTC) #8
commit-bot: I haz the power
7 years, 4 months ago (2013-08-17 04:34:40 UTC) #9
Message was sent while issue was closed. 
Change committed as 156261

Powered by Google App Engine
This is Rietveld 408576698