New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 5 users

Issue metadata

Status: Verified
Owner:
Email to this user bounced
Closed: Dec 2013
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Regression



Sign in to add a comment
link

Issue 330974: Text duplication on <bdi>

Reported by ebrami...@gmail.com, Dec 29 2013

Issue description

Chrome Version       : 33.0.1750.5
OS Version: 6.3
URLs (if applicable) : data:text/html,<div dir="rtl"><bdi>(5Hey I am not two </bdi>q</div>
Other browsers tested:
Add OK or FAIL after other browsers where you have tested this issue:
  Firefox 27: OK

What steps will reproduce the problem?
Open data:text/html,<div dir="rtl"><bdi>(5Hey I am not two </bdi>q</div>

What is the expected result?
You shouldn't see two same text

What happens instead of that?
You will see a duplicated text. Highlighting text on it is also interesting

Please provide any additional information below. Attach a screenshot if
possible.

UserAgentString: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/33.0.1750.5 Safari/537.36
 

Comment 1 Deleted

Comment 2 by ebrami...@gmail.com, Dec 29 2013

Comment 3 by ebrami...@gmail.com, Dec 29 2013

Available on

data:text/html;charset=utf-8,<bdi>%D8%B3%D8%B3</bdi>%D8%A7

data:text/html;charset=utf-8,<span style="unicode-bidi: -webkit-isolate;">(5%D8%B3%D8%B4 </span>%DB%8C

Comment 4 by ebrami...@gmail.com, Dec 29 2013

Interesting, Midori (Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-us) AppleWebKit/537+ (KHTML, like Gecko) Version/5.0 Safari/537.6+ Midori/0.4) is OK

Comment 5 by nyerramilli@chromium.org, Dec 30 2013

Cc: nyerramilli@chromium.org
Labels: -Type-Bug -Pri-2 Type-Bug-Regression Pri-1 M-31 Cr-Blink-RTL
Status: Untriaged
CHANGELOG URL:
  http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog.html?url=/trunk/src&range=223577%3A223608

Comment 6 by jeremy@chromium.org, Dec 30 2013

Cc: aharon@chromium.org
Labels: -OS-Windows OS-All

Comment 7 by aharon.l...@gmail.com, Dec 30 2013

Cc: le...@chromium.org eseidel@chromium.org igo...@sisa.samsung.com
This is a showstopper bug!!!

Google Web Search has just started using unicode-bidi:-webkit-isolate. Some other Google products have been using it (and relying on it) for months.

Reproduces on Version 31.0.1650.63 on Linux.
Reproduces on Version 32.0.1700.41 m Aura on Win7.
Reproduces on Version 34.0.1766.0 canary on Win7.

I am pretty sure that this is a regression, but cannot easily check earlier builds.

To reproduce, it seems that both of the following conditions have to hold:

1. The unicode-bidi:-webkit-isolate element contains more than one non-whitespace character. The directionality (or neutrality) of these characters does not matter.

2. The unicode-bidi:-webkit-isolate element is *immediately* followed by a strong character of the directionality opposite to the parent element's directionality, or, if the parent element is RTL, by a digit (makes no difference if EN or AN).

Comment 9 by jeremy@chromium.org, Dec 30 2013

Labels: ReleaseBlock-beat
Owner: igo...@sisa.samsung.com
Status: Assigned

Comment 10 by jeremy@chromium.org, Dec 30 2013

Labels: -ReleaseBlock-beat ReleaseBlock-Beta

Comment 11 by igo...@sisa.samsung.com, Dec 30 2013

I am on it.

Comment 12 by le...@chromium.org, Dec 30 2013

Thanks, Igor!

Comment 13 by igo...@sisa.samsung.com, Dec 31 2013

Comment 14 by bugdroid1@chromium.org, Jan 2 2014

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=164396

------------------------------------------------------------------------
r164396 | igor.o@sisa.samsung.com | 2013-12-31T09:12:45.743076Z

Changed paths:
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/text/international/unicode-bidi-isolate-duplicated-text-expected.html?r1=164396&r2=164395&pathrev=164396
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/text/international/unicode-bidi-isolate-duplicated-text.html?r1=164396&r2=164395&pathrev=164396
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/InlineIterator.h?r1=164396&r2=164395&pathrev=164396

When an isolated run is appended, m_eor(End Of Run) is wrongly incremented. 
Instead of moving to the next renderer, the iterator(m_eor) is just moving to the
next position in the same renderer.

BUG= 330974 

Review URL: https://codereview.chromium.org/122553005
------------------------------------------------------------------------

Comment 15 by aharon.l...@gmail.com, Jan 6 2014

Status: Verified
Thanks, Igor!

A whole spate of problems was discovered in this feature recently (many of them apparently dating back years to when it was originally developed). In your opinion, does the code look solid now, or should we expect more surprises?

Comment 16 by vclarke@chromium.org, Jan 7 2014

Cc: e...@chromium.org
Issue 331959 has been merged into this issue.

Comment 17 by vclarke@chromium.org, Jan 7 2014

Labels: -M-31 -ReleaseBlock-Beta M-32 ReleaseBlock-Stable
Can/should we merge this fix to M32 or M33? Adding M32 ReleaseBlock-Stable.

Comment 18 Deleted

Comment 19 by igo...@sisa.samsung.com, Jan 7 2014

Aharon,

Yeah I believe we are already in a good shape. Now we can focus in features  
instead of bug fixing!

Comment 20 by aharon.l...@gmail.com, Jan 7 2014

Yes, please, if at all possible, merge into M32.

Comment 21 by eseidel@chromium.org, Jan 7 2014

Labels: Merge-Requested
I believe the m32 ship has long sailed, but we can add Merge-Requested to have the merge-master tell us that officially.  I suspect this will have to ship in 33.

Comment 22 by kareng@google.com, Jan 7 2014

we've cut stable 1 aleady so it's too late but depending on how this does, we can consider it for stable 2.

Comment 23 by srsridhar@chromium.org, Jan 10 2014

Cc: srsridhar@chromium.org
Labels: Needs-Feedback
Tested the issue on Windows 8 and MacBook Pro 10.9 - canary version 34.0.1777.0 (Official Build 243882). Observed a deviation in the output when compared Chrome with Firefox browser.

Please find the attached screenshot for reference. Please confirm us on the fix.
Firefox.png
30.8 KB View Download
Canary.png
19.7 KB View Download

Comment 24 by ebrami...@gmail.com, Jan 12 2014

I confirm the original bug is fixed. This behavior however should be checked with standards likes http://www.unicode.org/reports/tr9/ . From what I know this is OK and Firefox should be corrected. IMO the whole point of <bdi> is making the content inside isolated like an object so in the below testcase b should on right.

data:text/html,<div dir="rtl"><img height="11" width="11"></img>b</div><div dir="rtl"><bdi>A</bdi>b</div>

Comment 25 by ebrami...@gmail.com, Jan 12 2014

* On left, on both lines

Comment 26 by ebrami...@gmail.com, Jan 12 2014

Another testcase that both line should be same IMHO:
data:text/html,<div dir="rtl"><span style="display: inline-block;">A</span>b</div><div dir="rtl"><bdi>A</bdi>b</div>
OK on Canary but differs from Webkit and Firefox and I guess http://src.chromium.org/viewvc/blink?revision=157874&view=revision is its bisect.

Comment 27 by aharon.l...@gmail.com, Jan 12 2014

Re comments 23 - 26:
- Canary's behavior is correct.
- FF behavior is a known bug (soon to be fixed in FF by going to the latest ICU implementation of UBA and implementation of unicode-bidi:isolate based on Unicode isolate formatting characters): https://bugzilla.mozilla.org/show_bug.cgi?id=717811
- Don't have a Mac handy right now, so unable to test WebKit.

Comment 28 by manoranj...@chromium.org, Jan 15 2014

Labels: -Needs-Feedback TE-Verified-34.0.1777.0 TE-Verified-M34
aharon@, Thanks so much for you confirmation.

Adding 'TE-Verified-34.0.1777.0' label as per comments# 23&27.

Thank you.

Comment 29 by kareng@google.com, Jan 17 2014

Labels: -M-32 M-33
let's merge to M33 first.

Comment 30 by laforge@google.com, Jan 18 2014

Labels: -Merge-Requested Merge-Approved
Merge approved for M33 (1750)

Comment 31 by laforge@google.com, Feb 5 2014

Owner: eseidel@chromium.org

Comment 32 by eseidel@chromium.org, Feb 14 2014

Sorry I was so slow to deal with this.

Committed revision 167218.

Comment 33 by bugdroid1@chromium.org, Feb 14 2014

Project Member
Labels: -Merge-Approved merge-merged-1750
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=167218

------------------------------------------------------------------------
r167218 | eseidel@chromium.org | 2014-02-14T19:35:35.635121Z

Changed paths:
   A http://src.chromium.org/viewvc/blink/branches/chromium/1750/LayoutTests/fast/text/international/unicode-bidi-isolate-duplicated-text-expected.html?r1=167218&r2=167217&pathrev=167218
   A http://src.chromium.org/viewvc/blink/branches/chromium/1750/LayoutTests/fast/text/international/unicode-bidi-isolate-duplicated-text.html?r1=167218&r2=167217&pathrev=167218
   M http://src.chromium.org/viewvc/blink/branches/chromium/1750/Source/core/rendering/InlineIterator.h?r1=167218&r2=167217&pathrev=167218

Merge 164396 "When an isolated run is appended, m_eor(End Of Run..."

> When an isolated run is appended, m_eor(End Of Run) is wrongly incremented. 
> Instead of moving to the next renderer, the iterator(m_eor) is just moving to the
> next position in the same renderer.
> 
> BUG= 330974 
> 
> Review URL: https://codereview.chromium.org/122553005

TBR=igor.o@sisa.samsung.com

Review URL: https://codereview.chromium.org/167513002
------------------------------------------------------------------------

Comment 34 by aharon.l...@gmail.com, Feb 17 2014

In comment 22, kareng@google.com wrote:

> we've cut stable 1 aleady so it's too late but depending on how this does,
> we can consider it for stable 2.

Will there be a 32 stable 2, and can we please merge this into it?

Comment 35 by ebrami...@gmail.com, Feb 9 2015

Just FWIW there is a somehow similar issue on  Issue 454099  with this testcase: data:text/html;charset=utf8,<span dir="auto"><span dir="rtl">%E0%A4%B7%E0%A5%8D (%D8%B3)</span></span>

Sign in to add a comment