100170 - Cluster based caret operations unsupported

Status: VERIFIED FIXED

(Core :: Internationalization, defect)

Description

[Prabhat Hegde]

IN languages such as Thai, Arabic and Indian languages, a single
display unit may consist of :

A> one or more (combining)characters
B> Presentation Forms/ligatures (One or more characters combining to form 
   a glyph)

In such cases cursor and text edit operations should consider a
"Cell/Cluster" as a display unit and not a 'character'. Currently,
mozilla does not support this feature.

A typical example of all the layout code that will break due to the
assumption of character=cell is :

A> Cursor Movements
B> Mouse Click (PointToContentOffset and ContentOffsetToStringIndex)
C> Selection

The following simple patch based on extensions/ctl should fix the 
problem:

Of course before this piece of code is enabled, we need to able to
build extensions/ctl on all platforms. (Currently have tried it on
Solaris and linux). Please let me know of your opinions on the 
patch while i try to add patches to get extensions/ctl built on
non *nix platforms.


Index: nsTextFrame.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/base/src/nsTextFrame.cpp,v
retrieving revision 1.318
diff -u -r1.318 nsTextFrame.cpp
--- nsTextFrame.cpp 2001/08/21 01:47:08 1.318
+++ nsTextFrame.cpp 2001/09/17 21:39:43
@@ -83,6 +83,10 @@
 //ahmed end
 #endif // IBMBIDI

+#ifdef SUNCTL
+#include "nsILE.h"
+#endif
+
 #ifndef PR_ABS
 #define PR_ABS(x) ((x) < 0 ? -(x) : (x))
 #endif

@@ -3894,12 +3929,31 @@
 #endif
         aPos->mContentOffset = 0;
     PRInt32 i;
+
+#ifdef SUNCTL
+    static NS_DEFINE_CID(kLECID, NS_ULE_CID);
+
+    nsCOMPtr<nsILE> mCtlObj;
+    mCtlObj = do_CreateInstance(kLECID, &rv);
+    if (NS_FAILED(rv)) {
+      NS_WARNING("Cell based Cursor Movement Will Not Be Supported\n");
+      mCtlObj = nsnull;
+#endif
         for (i = aPos->mStartOffset -1 - mContentOffset; i >=0;  i--){
       if (ip[i] < ip[aPos->mStartOffset - mContentOffset]){
             aPos->mContentOffset = i + mContentOffset;
             break;
       }
     }
+#ifdef SUNCTL
+    }
+    else {
+      PRInt32 mPreviousOffset;
+      mCtlObj->PrevCluster((const PRUnichar*)paintBuffer.mBuffer,
+               textLength, aPos->mStartOffset, &mPreviousOffset
);
+      aPos->mContentOffset = i = mPreviousOffset;
+    }
+#endif
         if (i <0){
       found = PR_FALSE;
       GetPrevInFlow(&frameUsed);
@@ -3915,12 +3969,34 @@
 #endif
         PRInt32 i;
     aPos->mContentOffset = mContentLength;
+
+#ifdef SUNCTL
+    static NS_DEFINE_CID(kLECID, NS_ULE_CID);
+
+    nsCOMPtr<nsILE> mCtlObj;
+    mCtlObj = do_CreateInstance(kLECID, &rv);
+    if (NS_FAILED(rv)) {
+      NS_WARNING("Cell based Cursor Movement Will Not Be Supported\n");
+      mCtlObj = nsnull;
+#endif
+
     for (i = aPos->mStartOffset +1 - mContentOffset; i <= mContentLength;
i++){
           if (ip[i] > ip[aPos->mStartOffset - mContentOffset]){
             aPos->mContentOffset = i + mContentOffset;
             break;
       }
     }
+
+#ifdef SUNCTL
+    }
+    else {
+      PRInt32 mNextOffset;
+      mCtlObj->NextCluster((const PRUnichar*)paintBuffer.mBuffer,
+               textLength, aPos->mStartOffset, &mNextOffset);
+      aPos->mContentOffset = i = mNextOffset;
+    }
+#endif
+
 /*  if (aStartOffset == 0 && (mState & TEXT_SKIP_LEADING_WS))
     i--; //back up because we just skipped over some white space. why skip
over the char also?
 */

prabhat

Comment 1

[Roy Yokoyama]

assigning to bstell.

Comment 2

[Kathleen :Brade]

This sounds like "Caret operations" (the blinking line indicating the insertion 
point) not "Cursor operations" (which is an arrow or other graphic which is moved 
around on a screen by a mouse)

Comment 3

[kill this account]

caret if one is using the arrow keys
cursor if one is using the mouse/pointer to select/set-insertion-point

Comment 4

[kill this account]

--> ftang

Comment 5

[Frank Tang]

smontagu- please take a look at this one.

Comment 6

[Frank Tang]

nsILE is not ready for all platform yet. move this to m98

Comment 7

[Simon Montagu :smontagu]

Marking fixed since the patch has been checked in (bug 84380).

Comment 8

[Arthit Suriyawongkul]

as tested with Mozilla 0.9.7 (Build ID: 2001122204)
with libraries from Prabhat's workspace
(/net/disk1.sfbay/space0/users/prabhath/netscape6 - 21 Dec)

seem likes it doesn't fixed yet.

ex.

legend:
 ABC   <-- english characters "ABC"
 <c>   <-- base level character  (e.g. consonant)
 <u>   <-- upper level character (e.g. upper vowel)
 <t>   <-- top level character   (e.g. tonemark)
 _     <-- caret position
 


input key strokes:
 <c><u><t>ABC


display order in vertical & horizontal:

<t>
<u>
<c>ABC

(<t> above <u>, <u> above <c>, A is next to <c>)

----

to move caret in front of <c> to back of A..
as tested in my environment, we need 4 left-arrow strokes.
like this...

(let <cut> represents <c><u><t> in vertical order)


stroke 0:  _<cut>ABC
stroke 1:  <cut>_ABC
stroke 2:  <cut>_ABC
stroke 3:  <cut>_ABC
stroke 4:  <cut>A_BC


the correct solution should be like this

stroke 0:  _<cut>ABC
stroke 1:  <cut>_ABC
stroke 2:  <cut>A_BC


just treat <cut> as one cell, one unit.


----

sorry if i've tested with the wrong build.
(pls inform me, which build i need to use, thx :) )

Comment 9

[Arthit Suriyawongkul]

for test case, try these short words

Comment 10

[Prabhat Hegde]

arthit,

Pl try testing with post 1/10 nightly or 0.9.8 builds. Note: you need to use
enable-ctl while building.

Comment 11

[Arthit Suriyawongkul]

VERIFIED, it has been fixed.
(i cannot mark it as VERIFIED by myself,
coz in my page view it shows only 1 selected option "Leave as RESOLVED FIXED")


Prabhat, pls see Bug 122552
"Caret cannot move all along the text, got stucked at some position".

Comment 12

[Arthit Suriyawongkul]

<left>,<right>,<backspace operations works fine.
expect <delete>.

REOPEN.

please see bug 157546
[CTL-Thai] IM: <delete> key should delete WHOLE Thai "display cell"

Comment 13

[Arthit Suriyawongkul]

sorry, i do mean.

<left>,<right>,<backspace> operations works fine.
_EXCEPT_ <delete>.

REOPEN.

please see bug 157546
[CTL-Thai] IM: <delete> key should delete WHOLE Thai "display cell"

Comment 14

[Masaki Katakai]

The exact problem of this bug has been fixed and I've
verified on my build (Solaris Netscape7 as well) with --enable-ctl. 
Should be markas as VERIFIED.

Comment 15

[Masaki Katakai]

verified.

Comment 16

[Masaki Katakai]

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

Comment 17

[Samphan Raruenrom]

The enable-ctl doesn't work on Windows build. This is tested for Firefox 1.0
build on Windows XP.
8<------------------------->8
configure: error: Cannot build ctl without gmodule support in glib.
8<------------------------->8

So the feature is lacking on Windows build. This feature is very important for
Thai (as well as other CTL languages) to work correctly in Mozilla applications.
And it should work for all platform. Should this issue be reopened and the
Hardware/OS changed to All/All or should I open another issue?

Comment 18

[Samphan Raruenrom]

I file a tracker bug for Windows cluster support :-
bug 283271 - Windows : CTL cluster-based operations unsupported on Windows
and a bug on caret movement
bug 283415 - Windows : Caret must be moved by grapheme cluster boundaries

Comment 19

[Dietrich Ayala (:dietrich)]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18704

WIP patch

Basics in place:

* List installed apps, touch to export
* List apps detected in backup/apps on SD card, touch to import

TODO:

* Needs to hook up to the not-yet-landed import/export APIs
* Needs some styling improvements
* Tests
* Error handling
* Confirmation prompts?

Comment 20

[Dietrich Ayala (:dietrich)]

Comment on attachment 8413024 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18704

Wrong bug, sorry!