16001 – REGRESSION: RTL selection is painted incorrectly

Bug 16001 - REGRESSION: RTL selection is painted incorrectly

Summary: REGRESSION: RTL selection is painted incorrectly

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	Layout and Rendering (show other bugs)
Version:	528+ (Nightly build)
Hardware:	Mac OS X 10.4

Importance:	P1 Normal
Assignee:	Nobody

URL:	data:text/html,%3Cdiv%20contenteditab...
Keywords:	InRadar, Regression

Depends on:
Blocks:	23386
	Show dependency tree / graph

Reported:	2007-11-15 07:02 PST by Alexey Proskuryakov
Modified:	2009-01-11 01:22 PST (History)
CC List:	3 users (show)

See Also:

Attachments
LayoutTests/editing/selection/extend-selection-bidi.html (807 bytes, text/html) 2008-12-25 20:42 PST, Jeremy Moskovich	no flags	Details
LayoutTests/editing/selection/extend-selection-bidi.html (799 bytes, text/html) 2008-12-25 20:49 PST, Jeremy Moskovich	no flags	Details
Patch w/Layout Test (8.51 KB, patch) 2008-12-27 08:37 PST, Jeremy Moskovich	darin: review+	Details \| Formatted Diff \| Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

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

Description Alexey Proskuryakov 2007-11-15 07:02:28 PST

Steps to reproduce:
1. Open the bug URL.
2. Focus the editable div.
3. Select 9 characters from the left (with shift-right arrow).

Expected results: "abc", "1" and "&#1488;&#1489;&#1490;" are selected.
Actual results: "&#1491;&#1492;&#1493;" appears selected, too, although with a different look. However, cut/delete work correctly.

Tested with release Safari/WebKit on 10.4.10 and 10.4.11.

Comment 1 mitz 2007-11-15 07:42:27 PST

The same thing happens with non-editable text. Any idea when the regression happened?

Comment 2 David Kilzer (:ddkilzer) 2007-11-28 22:16:12 PST

This bug appears to reproduce with the earliest known nightly, r11976, with Safari 2.0.4 on Mac OS X 10.4.11 (8S165).

Comment 3 mitz 2008-03-08 21:32:29 PST

<rdar://problem/5788857>

Comment 4 Xiaomei Ji 2008-12-18 16:34:12 PST

Also reported in Chrome:
http://code.google.com/p/chromium/issues/detail?id=3830

besides shift+arrow, shift+cntl+arrow does not work correctly either.

I am trying to add InChromeBugs in keywords, but I do not have the right permission.

Comment 5 mitz 2008-12-18 23:39:41 PST

(In reply to comment #4)
> Also reported in Chrome:
> http://code.google.com/p/chromium/issues/detail?id=3830

That bug is not related to this one. It is related to bug 3729 and covers cases not addressed by the patch for that bug.

Comment 6 Jeremy Moskovich 2008-12-25 20:42:59 PST

Created attachment 26250 [details]
LayoutTests/editing/selection/extend-selection-bidi.html

Layout Test

Comment 7 Jeremy Moskovich 2008-12-25 20:49:50 PST

Created attachment 26251 [details]
LayoutTests/editing/selection/extend-selection-bidi.html

Fix formatting.

Comment 8 Jeremy Moskovich 2008-12-27 08:37:24 PST

Created attachment 26268 [details]
Patch w/Layout Test

Make RootInlineBox::fillLineSelectionGap() not assume a contiguous visual LTR selection range.

Comment 9 Xiaomei Ji 2009-01-06 17:33:05 PST

Still think the following 2 Chrome bugs are related:

http://code.google.com/p/chromium/issues/detail?id=3830 (wrong highlight on pure RTL text selection)
and
http://code.google.com/p/chromium/issues/detail?id=4556 (wrong highlight on mixed RTL/LTR text selection)

Comment 10 Darin Adler 2009-01-10 15:40:56 PST

Comment on attachment 26268 [details]
Patch w/Layout Test

Does this regression test really work in an automated way, or is it only possible to judge its success or failure based on pixel results? Just curious.

r=me

Comment 11 Jeremy Moskovich 2009-01-10 17:41:25 PST

Thanks for looking at this Darin!

Since this is essentially a drawing bug and the surrounding data structures contain valid data, I'm not sure how to create a test for this which isn't pixel based.

Comment 12 Alexey Proskuryakov 2009-01-11 01:22:58 PST

Committed revision 39785.

Note that this patch didn't include pixel results, so I had to re-generate them (svn-create-patch script would have handled binary files correctly). Also, its a bit strange that results are not in platform directory, unlike other editing/selection results.