76190 - bidi- selection is broken in bidi enable build

Status: VERIFIED FIXED

(Core :: Layout: Text and Fonts, defect, P1)

Description

[Frank Tang]

I found this problem when I try to verify the Mac bidi enable build. This does 
not happen on Trunk. The Window BIDI enable build have the same problem.

It seems the text selection is not 100% broken, it seems work if the whole 
paragraph is ascii text without fancy format. But I see this kind of problem on 
text with a lot of format, or any CJK page. (for example: 
http://home.netscape.com/ja )

reproduce procedure
1. use bidi eanble build (mac or window, or linux)
2. go to resource:///res/samples/test0.html
3. put carret on the beginnig of the big "Size=7"
4. select down several lines and up several lines, you will see the selection is 
wrong.
5. the whole paragraph is english only, we should only see continuous selection, 
but I see discrete selection all the time.

Comment 1

[Frank Tang]

Attachment: broken selection

Comment 2

[Frank Tang]

remember, if you just select down, you probably won't see the problem, but if 
you select down and up, you will see the problem easily.

Comment 3

[Frank Tang]

I find the problem also happen on www.mozilla.org- 
way to reproduce there:
1. use bidi enable build
2. go to www.mozilla.org
3. put your carret in mid of a line
4. select down- you will see it somehow select from the beginning fo the line 
which you put the carret- this is wrong, it should start from the mid of the 
line
5. select up- it some how select from the carret to the end of that line (the 
correct one should select from the mid to the beginning of the line) and it 
select the above line regardless where is the mouse position. (it should only 
select up to the place of your mouse)

Comment 4

[Erik van der Poel]

Simon, has this bug always been there, or did I (or we) introduce it in our
recent changes? I tried a build from April 2nd, and the bug was there too. Do
you have any older builds?

Comment 5

[Simon Montagu :smontagu]

This is really 2 unrelated bugs. I'm fissioning them to make tracking easier.

1) Bidi selection code fired on non-Bidi pages -- created bug 76311 (with patch)
2) Bidi selection broken

Comment 6

[Frank Tang]

reassign to [e-mail]

Comment 7

[Ilya Konstantinov]

On http://www.msn.co.il/ (your typical Logical page), you cannot select multiple
lines of Hebrew text.
1. Place the mouse pointer on the first letter of the selection.
2. Hold the left button and drag the pointer left to some distance, and then
down / up some distance.

Expected result: Text is selected, from the first character to the character the
pointer was dragged to.

Actual result: No selection is made (when dragging the cursor between the lines,
the previously selected line gets unselected).

Comment 8

[Katsuhiko Momoi]

Adding Gilad to the CC line.

Comment 9

[Simon Montagu :smontagu]

*** Bug 94606 has been marked as a duplicate of this bug. ***

Comment 10

[Zach Lipton [:zach]]

Mass-move all BiDi Hebrew and Arabic qa to me, [e-mail]. 
Thank you Gilad for your service to this component, and best of luck to you 
in the future.

Sholom. 

Comment 11

[Sagie Maoz]

Selections on the resource page WFM on 2002042908/win2k.

However selecting Hebrew text in logical pages such as msn.co.il is still
broken. Is there a seperate bug for this?

Comment 12

[Simon Montagu :smontagu]

Attachment: Back out visual selection

I've spent some time during the last week or two debugging the remaining
problems with visual selection, and I don't believe I have the bandwidth to fix
them. We will be better off with working logical selection.

I'll file a new bug to re-implement visual selection.

Comment 13

[kinmoz]

Comment on attachment 97353 [details] [diff] [review]
Back out visual selection

sr=[e-mail]

Just add the "// VISUALSELECTION" comment for this endif to be consistent:


@@ -2764,6 +2773,7 @@
       mHint = saveHint;
     }
     else
+#endif
 #endif // IBMBIDI

Comment 14

[Ilya Konstantinov]

Why do you insist on adding visual selection at all? It's inconsistant with all
other BiDi software in existance, and from my Bidi experience, totally unusable. 

Don't see a reason I'd want my selection to mark a piece of Hebrew text and the
2 last (!) letters of a 5 letter English word that follows it. Do you see any
valid use for it?

Comment 15

[Simon Montagu :smontagu]

Personally I completely agree with comment 14, but I know that some people think
it's more important that the selected area on screen should be contiguous, which
requires visual selection.

I have also heard that on Mac OS 10.2 there is a system-wide setting for
logical/visual selection, which we should ideally be able to honour.

For those reasons, my patch leaves visual selection in the code, but #ifdef'd
out by default for now.

Comment 16

[Asa Dotzler [:asa]]

Comment on attachment 97353 [details] [diff] [review]
Back out visual selection

a=asa (on behalf of drivers) for checkin to 1.1

Comment 17

[Simon Montagu :smontagu]

Fix checked in. 

(I don't understand why r=mjudge isn't marked. I was standing right next to
mjudge and saw him update the attachment).

Comment 18

[mjudge]

Comment on attachment 97353 [details] [diff] [review]
Back out visual selection

better to work later than to be broken now. r= mjudge

Comment 19

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 97353 [details] [diff] [review]
Back out visual selection

a=[e-mail] for 1.0 branch checkin.  Please change mozilla1.0.2+ to
fixed1.0.2 when checked into branch

Comment 20

[Jaime Rodriguez, Jr.]

Carrying over edt1.0.2+ frombug 164201. Pls check this in asap, then replace
"Mozilla1.0.2" with "fixed1.0.2". thanks!

btw - Simon, can you have someone verify this as fixed on the trunk?

Comment 21

[Simon Montagu :smontagu]

*** Bug 164201 has been marked as a duplicate of this bug. ***

Comment 22

[Simon Montagu :smontagu]

Fix checked in to 1.0 branch.

Comment 23

[ji]

This is verified on 2002-09-09-04-trunk build, will verify the branch build
tomorrow.

Comment 24

[ji]

Verified as fixed on 09/11 1.0.2 branch.