299240 - BiDi: Caret gets stuck (or moves cyclically) when using Ctrl+arrow and reaching a reverse-direction character/word followed by punctuation.

Status: RESOLVED FIXED

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

Description

[Prognathous]

Tested with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2)
Gecko/20050629 Firefox/1.0+

To reproduce:

1. Open the attached testcase.
2. Put the caret to the right of the Hebrew letter Alef (first line, right corner)
3. Hold Ctrl+LeftArrow pressed.

Result: The caret moves recursively. Every time it reaches the end of the line
it gets back to the middle with the next key-press, as if the text never ends.

The LTR textarea doesn't seem to be effected by this bug.

Prog.

Comment 1

[Prognathous]

Attachment: Testcase B

Comment 2

[Abid]

*** This bug is a duplicate of big #299239 ***

Comment 3

[Abid]

*** This bug is a duplicate of bug #299239 ***

Comment 4

[Simon Montagu :smontagu]

This is not a duplicate. Compare the testcases and descriptions carefully, and
see also bug 207186 comment 41 and the following comments.

Comment 5

[Uri Bernstein]

Severety: major => normal

Comment 6

[Uri Bernstein]

Attachment: Another testcase

This is another (somewhat simpler) testcase which demonstartes essentially the
same bug.
In either of the textareas, place the caret at the beginning of either line.
Now try to use ctrl+left-arrow (in the RTL textarea) or ctrl+right-arrow (in
the LTR textarea) repeatedly. The caret gets stuck before the reverse-direction
word.

The key here is a word (or character) of "reverse" directionality, followed by
punctuation (a period, in this case).

This bug (as demonstrated both by the original testcase and by this one),
occurs only when layout.word_select.eat_space_to_next_word is set to true, i.e.
it only occurs by default on Windows (but not on Mac or Linux).

Comment 7

[Uri Bernstein]

Changing summary per comment #6.

Comment 8

[Uri Bernstein]

The patch for bug 331444 will make fixing this bug easy (I'll post a patch here if/when the patch for 331444 lands).

Comment 9

[Uri Bernstein]

Attachment: patch

The actual fix (for this bug, as well as bug 299531) is in the last block: the test should be for direction relative to this frame, not relative to the block (just like we do several times above).

The rest of the patch is just cleanup:
- compute the direction-relative-to-frame (movingInFrameDirection) once, at the beginning, instead of each time.
- Get rid of two unused variables.

ROC, try to think of it as code cleanup, with the bug fix as a bounus :)

Comment 10

[Uri Bernstein]

s/movingInFrameDirection/movementIsInFrameDirection/, according to Simon's suggestion.

Checked in:
Checking in layout/generic/nsTextFrame.cpp;
/cvsroot/mozilla/layout/generic/nsTextFrame.cpp,v  <--  nsTextFrame.cpp
new revision: 1.564; previous revision: 1.563
done