188288 - [patch] caret movement erratic with CTL enabled

Status: RESOLVED FIXED

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

Description

[Louie Zhao]

mozilla (both gtk and gtk2 build) with "--enable-ctl" will cause caret to
behavior disorderly.It's quite severe since it always happen when user compose a
mail.

Reproduce steps:

1. open mailnews and compose a mail using html format
2. type a paragraph without hit return which can be automaticly wrapped into
more than one line.
3. use left arrow key to move back

expect, the caret moves back
result, the caret runs to some unexpect place ( for example, directly to the
behind of the 5th charact of the first line)

Comment 1

[Prabhat Hegde]

This is due to an error in logic of:
http://lxr.mozilla.org/seamonkey/source/intl/ctl/src/nsULE.cpp#305.

Here is what i am planning to do. Use the basic-shaper to handle all scripts
other than CTL in pangolite. This will clean up the logic in handling caret
positions. This will require some re-write and testing of nsULE and also
addition of new shaper but should be done by next week.

prabhat

Comment 2

[Prabhat Hegde]

Attachment: Patch to fix this problem

The patch does the following:
A> Fixes a logical bug in CTL cursor placement logic.
B> Prevents CTL cursor positioning logic from being used when the endpoints of
an  
   edit operation do not require CTL processing. Whether or not a script
requires 
   Complex processing is decided based on if it has a pangolite shaper. This
fix 
   also has the side effect of speeding up non-CTL cursor positioning.

Note that this patch sits on top of patch for bugzilla #184599.

Comment 3

[Hixie (not reading bugmail)]

*** This bug has been marked as a duplicate of 122879 ***

Comment 4

[Roland Mainz]

Prabhat Hegde:
Should we reopen this bug or move your patch to bug 122879 ?

Comment 5

[Roland Mainz]

Reopening bug since this one has a patch...

Comment 6

[Roland Mainz]

Comment on attachment 115149 [details] [diff] [review]
Patch to fix this problem

prabhat:
The patch is slightly rotten - can you try to update it to "trunk", please ?

Comment 7

[Jungshik Shin]

Attachment: patch updated

I updated attachment 115149 [details] [diff] [review], got rid of tabs and cleaned up other white spaces.

Comment 8

[Jungshik Shin]

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

Comment 9

[Jungshik Shin]

let's move it to layout/ctl for a better tracking. I18N can be tracked with
'intl' keyword.

Comment 10

[Arthit Suriyawongkul]

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

Comment 11

[Arthit Suriyawongkul]

selection (Bug 100173) and highlighting (Bug 167983) also broken

Comment 12

[Jungshik Shin]

Attachment: udpdated patch

There was a typo in the previous patch I didn't notice because of the way I
compiled. Anyway, with this patch, it seems to work fine for non-complex
scripts. I have to hunt for an Xkb map for Devanagari or Thai to test it fully.


Here, I also removed 'm'-prefix used for local variables. Mozilla-convention
(not everywhere but certainly in nsTextFrame.cpp) is to use 'm' prefix for
member variables only.

Comment 13

[Jungshik Shin]

Comment on attachment 138248 [details] [diff] [review]
patch updated

.

Comment 14

[Steven Lumos]

So did this patch make it into the Solaris build of 1.6, or do I need to build
my own without --enable-ctl?

Comment 15

[Roland Mainz]

Comment on attachment 138278 [details] [diff] [review]
udpdated patch

Requesting r=/sr= to get some progress here. Rumor says we have little more
than a week until 1.7a and I'd like to see this problem fixed for that release.

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Could you attach a diff -ub as well?  (Ignores indentation changes.)

Comment 17

[Roland Mainz]

Attachment: Updated patch for 2004-02-22-trunk

Comment 18

[Roland Mainz]

Attachment: Updated patch for 2004-02-22-trunk (same as attachment 141978 but diff'ed with -u -w to ignore whitespace changes)

Comment 19

[Roland Mainz]

Comment on attachment 141978 [details] [diff] [review]
Updated patch for 2004-02-22-trunk

r=[e-mail] (original patch was from prabhat). The patch
compiles and fixes the problem... :)

Comment 20

[Nils T Siebel]

(In reply to comment #14)
> So did this patch make it into the Solaris build of 1.6, or do I need to
> build my own without --enable-ctl?

No, I am afraid the bug is still there in 1.6 and you will have to compile it
yourself.  The standard 1.6 build for sparc-sun-solaris2.8 is built with
"--disable-tests --disable-debug --with-xprint --enable-xinerama --enable-ldap
--enable-crypto --enable-x11-shm --enable-ctl" .

Kind regards,

Nils.

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 141978 [details] [diff] [review]
Updated patch for 2004-02-22-trunk

+	   static NS_DEFINE_CID(kLECID, NS_ULE_CID);

These should really be hoisted to file scope.

sr=roc whether you do that or not

Comment 22

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in layout/html/base/src/nsTextFrame.cpp;
/cvsroot/mozilla/layout/html/base/src/nsTextFrame.cpp,v  <--  nsTextFrame.cpp
new revision: 1.453; previous revision: 1.452
done


(with the DEFINE_CID moved directly after the #include in the ifdef SUNCTL block)

Comment 23

[Roland Mainz]

I have redone the Mozilla 1.7a release builds for Solaris with this patch
applied:
MD5
(http://ftp.mozilla.org/pub/mozilla.org/mozilla/releases/mozilla1.7a/contrib/mozilla-sparc-sun-solaris2.8-1.7a.tar.gz)
= 3174bae0d6e4a22d098936ae67505264
MD5
(http://ftp.mozilla.org/pub/mozilla.org/mozilla/releases/mozilla1.7a/contrib/mozilla-sparc-sun-solaris2.8-gtk2-1.7a.tar.gz)
= a67554ba5f61c1c2c11788545046a52d

Can anyone please verify that the issue is fixed in these builds ?

Comment 24

[Andreas Lange]

(In reply to comment #23)
> mozilla-sparc-sun-solaris2.8-gtk2-1.7a.tar.gz)
> Can anyone please verify that the issue is fixed in these builds ?

Just tested it a little, but it seems fine. It was quite easy to reproduce before.

Comment 25

[Roland Mainz]

Andreas Lange wrote:
> > mozilla-sparc-sun-solaris2.8-gtk2-1.7a.tar.gz)
> > Can anyone please verify that the issue is fixed in these builds ?
>
> Just tested it a little, but it seems fine. It was quite easy to reproduce 
> before.

OK... should I respin the mozilla1.6 release builds incl. this patch, too ?

Comment 26

[Meik Langwald]

I just tested the build from Roland (see comment #23)

The erratic behavior of the carret seems to be gone in mailnews composer windows
and in HTML form TEXTAREA boxes. 

But I encountered another problem, for which I'm not sure, if it's a problem of
the bug fix or of 1.7a. Copying text from an CDE window (dtterm Terminal or
dtpad Editor) doesn't work any more. 

Here are some steps to reproduce that problem: 
- open a mailnews composer window (I use the plan ascii version)
- mark some text from a CDE dtterm 
- try to copy the marked text by pressing the middle button into the composer window

Result: text is not copied. 

You can try it by using the "Edit --> Copy" and "Edit --> Paste" functions in
both dtterm and mozilla, but that has the same effect. 

Copying text between two mozilla windows works as expected. 

Can somebody please verify this? Is this a known bug in 1.7a? (I kept 1.6 until now)

I'm currently sitting at a SunRay terminal connected to a Solaris 9 box, but I
don't think this could be the problem. I'm also currently applying the
recommended patches for mozilla to a box with Solaris 8 and retry the test. 
I'll try the original mozilla 1.7a build for solaris on that box, too. 

Comment 27

[Roland Mainz]

Meik Langwald wrote:
> I just tested the build from Roland (see comment #23)
>
> The erratic behavior of the carret seems to be gone in mailnews composer 
> windows and in HTML form TEXTAREA boxes. 
>
> But I encountered another problem, for which I'm not sure, if it's a problem 
> of the bug fix or of 1.7a. Copying text from an CDE window (dtterm Terminal or
> dtpad Editor) doesn't work any more. 

That issue is not related to CTL(=Complex Text Layout) ... what you're seeing
is  bug 233317 ("Cannot paste from clipboard") ... ;-(