User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20031007 Firebird/0.7 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20031007 Firebird/0.7 When using Shift+Arrow keys to select text in the presence of dir="rtl" text, the behaviour of the selection boundary movement is strange. For example, after clicking in the Arabic text in the url mentioned above, repeatedly hitting Shift+Left will select toward the end of the Arabic text (to the left), but then will wrap around to the end of the word "Arabic", and repeat. Similarly, Shift+Right will select toward the beginning of the Arabic text, then suddenly the selection will jump to the beginning of the document. Reproducible: Always Steps to Reproduce: 1. Click once on Arabic text. 2. Use Shift+Left or Shift+Right to select text. 3. Continue until an edge of the Arabic text is reached. Actual Results: The selection boundaries jump around in unexpected ways depending on which direction you're going. Expected Results: When moving left, the beginning selection boundary should move up to the end of the previous line. When moving right, the end selection boundary should continue into the word "Arabic". This behaviour would match click-and-drag mouse movements in the same direction as the arrow keys.
Prog: you have been dealing with similar bug. Can you confirm or dup this one?
I can only think of two bugs that I opened and might be related: Bug 207186 - "Moving the caret in BiDi texts is buggy, especially in right-to-left forms" Bug 209430 - "Ctrl+Delete and Ctrl+BackSpace delete words in the wrong direction" BTW, I did manage to reproduce this behavior using the latest 1.6b nightly, but I'm not sure that it's not by design. Mozilla doesn't use the same algorithm as Windows for arrowing the caret through BiDi texts. Ask Simon Montagu for more details. Prog. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031212
Actually, this bug does have similarities with the first testcase that I provided in bug 207186, comment 1 (-> the "endless loop"). Prog.
I see this also on LInux 2003122007
After creating a reduced testcase of this behavior, I found out that it is not about text selection, but more about caret movement - quite similar to the behavior described in bug 207186, comment 1. I will now add this testcase there, and confirm this one as a NEW bug that depends on a fix for 207186. Also, Layout/BiDi seems to be the more fitting Component for this bug. Prog.
Created attachment 187514 [details] testcase This is easiest to see in Caret Browsing mode (use F7): Place the caret at the very left of the RTL cell, and press the left arrow. Expected: The caret moves out of the table, to the right of "228290". Observed: The caret moves to the very left of the LTR cell. Place the caret at the very right of the RTL cell, and press the right arrow. Expected: The caret moves to the very left of the LTR cell. Observed: The caret moves to the beginning of the document(!)
Seeing on OS X => All/All.
Ginn, please see comment #6. Thanks, Prog.
Prog, I think it is a dup of 207186.
(In reply to comment #9) > Prog, > > I think it is a dup of 207186. No, I don't think it's a dup of any of the issues currently reported in 207186, and I'd rather keep it separate instead of it getting lost as comment #456 or something on that bug. Also - assigning to myself (this looks fun, I'll give it a try).
Created attachment 187672 [details] [diff] [review] patch This patch contains two separate fixes, both required to fully fix this bug. The main fix is in nsFrameTraversal.cpp, to nsVisualIterator. In order for the iterator to really be visual, the direction of traversing the (logical) frame tree must be reversed at points where the directionality of the "current" frame is opposite to the directionality of the original frame. This affects two places in Next(): selecting the next (visual) sibling becomes selecting the previous sibling if the direction is reversed, and selecting the first child becomes selecting the last child. For Prev() the situation is reversed. Confused? So was I. But I believe I got it right eventually. The second fix, in nsFrameList.cpp, is much simpler. The visual methods should treat the various types of table-related frames like they treat block frames (i.e., just return the prev/next logical sibling). There might be more types of frames which need to be handled this way, and there probably is a better way of doing this (without enumerating all these types). However, I couldn't find one.
Created attachment 188059 [details] [diff] [review] patch v. 2 Actually, we need to decide on the "basic" direction once, based on the direction of the initial element - not the current element whenever Next() or Prev() are called. The previous patch caused lockups in certain situations, and worked poorly in others.
Created attachment 188061 [details] [diff] [review] patch v. 2 Errr... sorry about that. The patch I posted a few minutes ago included a fix for another bug.
Created attachment 194422 [details] [diff] [review] patch v. 2.1 I realized that this also fixes the underlying problem in bug 274857 and bug 295142, so this version of the patch also removes Ginn's workaround for these two bugs.
Created attachment 196913 [details] [diff] [review] patch v. 3 Changes from v. 2.1: - Added comments explaining how nsVisualIterator::Next/Prev() work. - Do the "drill down to first (last) descendant" part of the algorithm visually: -- Take into account the direction of each parent node on the way. -- Added methods to nsFrameList for getting the first and last visual frames in the list, and used them during the drilling down. - Use IsContainingBlock() to determine whether a frame is a block (or table-related) frame. - Fix the problem described in bug 309432 comment #4: In GetNextVisualFor(), only return the next logical sibling if the current frame *and the next frame* are both block frames (same for GetPrevVisualFor, current *and previous* frame). - Various nits picked by Simon on IRC
Comment on attachment 196913 [details] [diff] [review] patch v. 3 Mmm... no. I obviously need to get a better understanding of how how bidi really works before continuing to work on this.
Created attachment 215570 [details] better testcase This testcase is more similar to the original page this bug was reported to, and is therefore better than the artificial testcase I attached earlier.
(In reply to comment #0) > Shift+Right will select toward the beginning of the Arabic text, then suddenly > the selection will jump to the beginning of the document. This problem, which might be the only actual valid bug here, was fixed by the patch for bug 303884 (and its follow-up, bug 330284). As for the other issues reported, I now tend to consider them INVALID, or at least WONTFIX. Our caret and selection behaviour can be summarized as "visual inside blocks, logical between blocks", that is, moving forwards past the end of a block, the caret (or selection), always arrives at the next (logical) block; moving backwards past the beginning of a block, the caret always arrives at the previous block. (Exactly where in the previous/next block the caret ends up is something that is going to be changed by the patch to bug 330815, if it is approved). This is what happens here when you move left in the RTL (Arabic) cell. When you move left (forwards) past the leftmost (last) letter, the caret/selection moves into the next cell, which is the one containing the word "Arabic". Similarly, when moving right (backwards) past the rightmost (first) Arabic letter, the caret/selection moves into the previous cell ("English"). The question is whether we want to change this behaviour in the special case where the blocks in question are table cells. I tend to think the answer is "no", but I'll leave this open for discussion.
is selecting 2 farsi lines at this page the same problem? or should I file another bug? http://ubiquity.mozilla.com/trac/wiki/TracUnicode
This bug is about tables, so that is a different problem. (It seems to be mostly caused by the Persian lines not having right-to-left direction set, so I'm not sure that it is a bug in Firefox at all)