Last Comment Bug 307533 - [FIX] Caret drawn at incorrect position after typing a single LTR character in a blank RTL input field (or vice versa) while caret is visible
: [FIX] Caret drawn at incorrect position after typing a single LTR character i...
Status: RESOLVED FIXED
: fixed1.8, regression, rtl, testcase
Product: Core
Classification: Components
Component: Layout: Text (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Mats Palmgren (:mats)
:
Mentors:
Depends on:
Blocks: 296464
  Show dependency treegraph
 
Reported: 2005-09-08 10:58 PDT by Uri Bernstein (Google)
Modified: 2008-07-31 02:43 PDT (History)
8 users (show)
asa: blocking1.8b5+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
input boxes (104 bytes, text/html)
2005-09-08 11:00 PDT, Uri Bernstein (Google)
no flags Details
Patch rev. 1 (4.32 KB, patch)
2005-09-08 15:56 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Review
Patch rev. 1 (diff -w) (3.72 KB, patch)
2005-09-08 15:59 PDT, Mats Palmgren (:mats)
uriber: review+
bzbarsky: superreview+
asa: approval1.8b5+
Details | Diff | Review
Possible regression example (3.98 KB, image/jpeg)
2005-09-10 06:14 PDT, Mark Perris
no flags Details

Description Uri Bernstein (Google) 2005-09-08 10:58:25 PDT
Steps to reproduce:
1. Place the caret in a blank RTL input field or textarea
2. While the caret is visible (i.e. during the "on" half of the caret blink
cycle), type a single LTR (English) character.
3. The caret will appear to the left of the typed character, instead of to its
right.
4. Deleting this character will now require two presses of "backspace".

The same happens when typing a single RTL (Hebrew) character into a blank LTR
input field or textarea.
This does not happen if the character is typed while the caret is not visible
(i.e. during the "off" half of the blink cycle).

This regressed between 2005-08-10 and 2005-08-11, probably due to the checkin
for bug 296464.
Comment 1 Uri Bernstein (Google) 2005-09-08 11:00:36 PDT
Created attachment 195293 [details]
input boxes

Input boxes (RTL and LTR) for reproducing the bug.
Comment 2 Boris Zbarsky [:bz] 2005-09-08 12:15:08 PDT
Is this something important enough we want it fixed on branch?
Comment 3 Uri Bernstein (Google) 2005-09-08 12:33:52 PDT
(In reply to comment #2)
> Is this something important enough we want it fixed on branch?

I wasn't sure before, but I just found out that this happens not only in a blank
field, but also every time a first reverse-direction character is typed
following a sequence of paragraph-direction text. So this will actually happen
quite often.

It's still not major, but it is yet another annoyance to bidi editing. I've been
trying to get rid of this kind of bugs, and seeing new ones appear is somewhat
frustrating. So I would very much like to see this fixed on the branch, unless
the fix is especially complicated or risky.
Comment 4 Mats Palmgren (:mats) 2005-09-08 15:54:38 PDT
I didn't realize that the calculation of the frame position was depending
on the bidi level state which could change between draw and erase time...
Comment 5 Mats Palmgren (:mats) 2005-09-08 15:56:36 PDT
Created attachment 195324 [details] [diff] [review]
Patch rev. 1
Comment 6 Mats Palmgren (:mats) 2005-09-08 15:59:06 PDT
Created attachment 195325 [details] [diff] [review]
Patch rev. 1 (diff -w)

Save the bidi level that was used to when we draw the caret and use that when
we erase.
Comment 7 Mats Palmgren (:mats) 2005-09-08 16:02:17 PDT
I have only tested the RTL input. I don't know how to input a Hebrew character
on my keyboard. Uri, can you test that bit please?
Comment 8 Uri Bernstein (Google) 2005-09-09 00:14:35 PDT
Comment on attachment 195325 [details] [diff] [review]
Patch rev. 1 (diff -w)

The patch looks good.
I wonder why the caret bidi level is computed inside DrawAtPositionWithHint()
and not passed into it like the other parameters - but that's probably outside
the scope of this bug (I might want to change it for bug 305798).

I tested this with the LTR and RTL cases and it works in both of them.

Thanks for the quick fix, Mats. Given that this is relatively simple, I suggest
that you request approval1.8b5.
Comment 9 Mats Palmgren (:mats) 2005-09-10 05:03:28 PDT
Checked in to trunk at 2005-09-10 04:14 PDT
Comment 10 Mats Palmgren (:mats) 2005-09-10 05:04:35 PDT
Comment on attachment 195325 [details] [diff] [review]
Patch rev. 1 (diff -w)

low risk fix for a regression
Comment 11 Mark Perris 2005-09-10 06:14:08 PDT
Created attachment 195532 [details]
Possible regression example

Could this be a possible cursor glitch caused by this fix?

After entering text (LTR) into either field, and then moving the cursor to the
left or right of the said text, a small ~2 pixel artifact appears near the top
of the I-Bar (See the attatchment). This does not seem to happen on boxes that
are not tagged with specific LTR or RTL properties.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050910
Firefox/1.6a1
Comment 12 Mats Palmgren (:mats) 2005-09-10 06:40:38 PDT
(In reply to comment #11)
>  ... a small ~2 pixel artifact appears near the top of the I-Bar ...

That's the bidi marker and it should be there.
Comment 13 Mats Palmgren (:mats) 2005-09-13 21:05:36 PDT
Checked in to MOZILLA_1_8_BRANCH at 2005-09-13 20:41 PDT

-> FIXED
Comment 14 :Ehsan Akhgari (busy, don't ask for review please) 2008-04-07 13:58:11 PDT
Mass-assigning the new rtl keyword to RTL-related (see bug 349193).

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