Arthit Suriyawongkul Reporter
	Description • 23 years ago

when we using <left-arrow> / <right-arrow> key to move caret along the text,
it will got stuck at some unpredictable positions, prevented us to move the
caret to the end of text.
----

e.g.
given a sample text (Thai, TIS-620 encoded)
[begin]
╥крц Ю╧ Ю ║
 Ю мр╗Б║╖  ЮбИ
[end]

1. put the caret at the starting of the text (in front of ╥крц...)
2. press <right-arrow> key repeatingly, to move caret right to the end of text.
3. it will got stucked at some position.
    from the above case, this is what i've found
    ( "_" is the position of caret, when it got stucked)
      ╥кр_ц Ю╧ Ю ║
      ╥крц Ю_╧ Ю ║
      ╥крц Ю╧ Ю_ ║
      ╥крц Ю╧ Ю _║
      ╥крц Ю╧ Ю ║_     <--- cannot go further to next line

----

this issue do sometimes occurs with English text (ISO-8859-1)

	Rui Xu
	Comment 1 • 23 years ago

Arthit: Thank you for reporting this bug.

Could you please let me know:
1. Which build are you using?
2. Is this defect in browser? or in Composer? or in etc...
3. Do you see the similar problem on Windows, or Mac, or Linux, or etc.?

	Arthit Suriyawongkul
	Comment 2 • 23 years ago

1. i found it in internal build that Masaki@japan.sun have sent to me (build no.
is 0000000000!!).
   anyway, also defected in moz 0.9.7 and solaris nightly build 20020115xx.

2. occurs in every apps - browser (in html form), composer, mail

3. will try it

	Arthit Suriyawongkul Reporter
	Comment 3 • 23 years ago

to answer Rui Xu's 3rd question:
i've try this with 0.9.8
- on Windows 2000, not occurs, caret move smoothly.
- on Solaris 8, occurs, caret stucked.

	Theppitak Karoonboonyanan
	Comment 4 • 23 years ago

I find this similar bug in 0.9.8 (--enable-ctl).
It seems the caret gets stuck at "word boudaries."

	Theppitak Karoonboonyanan
	Comment 5 • 23 years ago

Sorry for my previous imprecise comment.

I found that, moving right, the caret is always stuck before every last
character before spaces,
except when the last cell before space is more than one character long, e.g.

à»ç|¹ Á¹ØÉÂì ÊØ|´ »ÃÐàÊÃÔ|° àÅÔ|È ¤Ø³¤èÒ

(where the pipes '|' represent the stuck places)

	Theppitak Karoonboonyanan
	Comment 6 • 23 years ago

Attached is a patch that fixes the problem on my box.

	Prabhat Hegde Assignee
	Comment 7 • 23 years ago

hi theppitak,

Thanks for looking at the bug. Yes the patch fixes the problem. r= from me for
nsULE.cpp. Will get masaki to check in.

prabhat.

	Frank Tang
	Comment 8 • 23 years ago

give to katakai

	Frank Tang
	Comment 9 • 23 years ago

katakai, can you ask sun's browser team to work on this if sun care a lot about
thai.

	Prabhat Hegde Assignee
	Comment 10 • 23 years ago

Stopped by the --enable-ctl bustage (#133212). In any case the fix will 
initially be available in the Sun builds until we figure out how to get
--enable-ctl going by default again.

	Arthit Suriyawongkul Reporter
	Comment 11 • 23 years ago

major loss of function,
severity --> MAJOR.

	Pete Zha
	Comment 12 • 22 years ago

I'd like to check this in.
Could anyone give r= and sr= ASAP?

	Arthit Suriyawongkul Reporter
	Comment 13 • 22 years ago

could anybody help Pete Zha ?

	Arthit Suriyawongkul
	Comment 14 • 22 years ago

since bug 133212 "--enable-ctl bustage" has been fixed.

could anyone help review Theppitak's patch pls? thanks.

	Masaki Katakai
	Comment 15 • 22 years ago

I tried the patch but it's not working properly for me,
I'm sorry I don't understand Thai language so I'm not sure
the example is correct or not, but please try the following,

¿¿¿¿··èÓÓèÓè¡Ó¡è¡¡è

1.move caret to the end of line
2.hit left-arrow key
  cursor jumps to "¡Ó" as one character, which means cursor
  is not moved to between the two characters
  Also a curet does not move to between "·èÓ"
3.After curet moved to beginning of the line,
  hit right-arrow key
  cursor stops at before "Ó"

But the patch works for the examples of Arthit and Theppitak.

	James Clark
	Comment 16 • 22 years ago

I don't think the behaviour Masaki is complaining of is a bug.  What's happening
is that a consonant together with a following sara-am as a cluster.  Uniscribe
does exactly the same thing.  Let me try to explain why this is reasonable
behaviour.  Sara am is a weird case because graphically it is the combination of
nikhahit (which is a combining char) and sara aa (which is a base char).
However, Thais typically think of it as an indivisible logical unit: they would
type it and spell as one char not two.  Consider what happens if you don't treat
the consonant plus sara am as a cluster: visually, a vertical cursor in the
middle of a consonant+sara am sequence would have to have the consonant plus the
nikhahit to its left and the sara aa to its right.  You would be faced with two
choices for the logical position of the cursor both of which cause problems:
- you decompose the sara am into nikhahit and sara aa and logically consider the
cursor to be between the two positions; this is bad because Thai's don't
typically decompose sara am; rather they think of it as a single indivisible unit
- you logically consider the cursor to be between the  consonant and the sara
am; this is bad, because the visual appearance doesn't match the logical situation
Not splitting the consonant plus sara am may be a little strange, but I think
it's way better than the alternatives.

BTW, I see this on Linux: I don't think there's anything Solaris specific.  Also
perhaps this should be reassigned to ehe Complex Text Layout component.

	James Clark
	Comment 17 • 22 years ago

Here's an alternative, minimal patch to fix the bug.

I had a look at the earlier patch.  It makes the assumption that the glyphs in
aGlyphData are in 1-1 correspondence with the characters in the run.  Although
this is probably the case with Thai and perhaps always with pangolite, it's not
in general the case with pango and the rest of the code in nsULE.cpp appears
not to make this assumption.  So I think it's best to keep the original logic
and not to make this assumption.

My patch just fixes the logic bug in the code without changing the algorithm. 
The bug is that in the last pass through the for loop, in the case when the
last glyph is the start of a cluster, the else clause sets end to start of the
logical cluster (which will be equal to j) rather than to the end of the run. 
The patch ensures that if it does not find a suitable cluster start point that
is past aIndex then it will use the end of the run regardless of whether the
last glyph starts a cluster.

	Masaki Katakai
	Comment 18 • 22 years ago

Prabhat,

Could you take this bug? 

	Prabhat Hegde Assignee
	Comment 19 • 22 years ago

James or Art,
I worked on performace enhancing the CTL shapers and also cursor positioning
logic. That seems to have also fixed this bug. Could one of you verify with
patch #id=121744 or bugzilla #id=203406. If applying this huge patch is an issue
and you need the entire enchilada, email me.

thanks,
prabhat

	Martijn Wargers (dead)
	Comment 20 • 17 years ago

Is this still an issue in current trunk build?

	Arthit Suriyawongkul
	Comment 21 • 17 years ago

Tested with Firefox 3 Beta 3, on GNU/Linux (Ubuntu 8.04 Alpha 4), the described bug is gone.

Using testcase from comment #5

in a correct encoding, the text in testcase should be read:

เป็|น มนุษย์ สุ|ด ประเสริ|ฐ เลิ|ศ คุณค่า


you can try test it here, to see if it got stucked or not:

(from comment #5):  เป็น มนุษย์ สุด ประเสริฐ เลิศ คุณค่า

(w/ additional cases):  เป็น มนุษย์ สุด ประเสริฐ เลิศ คุณค่า ฤๅฐูฎูนี้นั้นนิ์

	Arthit Suriyawongkul
	Comment 22 • 17 years ago

may got fixed by patch from bug 157546 ?

	Arthit Suriyawongkul
	Comment 23 • 17 years ago

(In reply to comment #22)
> may got fixed by patch from bug 157546 ?
> 

sorry, I mean "may able to use the Mochitest testcase provided in bug 157546 ?".


If Windows and Mac OS X ppl agree,
I think we can close this bug.

	Keng Susumpow
	Comment 24 • 17 years ago

Tested with Mac OS X 10.5.2 on Minefield 3.0b4pre build 2008022104 with the given test cases on Comment #21 seems this bug has gone from trunk.

If someone on windows can confirm this bug has gone, I think this bug can be closed.

	Arthit Suriyawongkul
	Comment 25 • 17 years ago

I see no problem with Windows.
(actually it had never been a problem with Windows before.)

close as FIXED. reopen if necessary.

	Martijn Wargers (dead)
	Comment 26 • 17 years ago

It's not clear what fixed, so marking worksforme.