If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[patch] caret movement erratic with CTL enabled

RESOLVED FIXED

Status

()

Core
Layout: Text
--
major
RESOLVED FIXED
15 years ago
9 years ago

People

(Reporter: Louie Zhao, Assigned: Prabhat Hegde)

Tracking

(Blocks: 1 bug, {intl})

Trunk
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

15 years ago
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)

Updated

15 years ago
Keywords: intl
(Assignee)

Comment 1

15 years ago
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
(Assignee)

Comment 2

15 years ago
Created attachment 115149 [details] [diff] [review]
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.

*** This bug has been marked as a duplicate of 122879 ***
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → DUPLICATE

Comment 4

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

Comment 5

15 years ago
Reopening bug since this one has a patch...
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

Comment 6

15 years ago
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 ?

Updated

15 years ago
Blocks: 122879

Comment 7

14 years ago
Created attachment 138248 [details] [diff] [review]
patch updated

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

Updated

14 years ago
Attachment #115149 - Attachment is obsolete: true

Comment 8

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

Comment 9

14 years ago
let's move it to layout/ctl for a better tracking. I18N can be tracked with
'intl' keyword.
Blocks: 201746
Component: Internationalization → Layout: CTL
Summary: the move of caret by arrow keys is disordered due to "--enable-ctl" → [patch] caret movement erratic with CTL enabled

Updated

14 years ago
Attachment #138248 - Flags: superreview?(rbs)
Attachment #138248 - Flags: review?(prabhat.hegde)

Comment 10

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

Comment 11

14 years ago
selection (Bug 100173) and highlighting (Bug 167983) also broken

Comment 12

14 years ago
Created attachment 138278 [details] [diff] [review]
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.
Attachment #138248 - Attachment is obsolete: true

Comment 13

14 years ago
Comment on attachment 138248 [details] [diff] [review]
patch updated

.
Attachment #138248 - Flags: superreview?(rbs)
Attachment #138248 - Flags: review?(prabhat.hegde)

Comment 14

14 years ago
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

14 years ago
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.
Attachment #138278 - Flags: superreview?(dbaron)
Attachment #138278 - Flags: review?(prabhat.hegde)
Could you attach a diff -ub as well?  (Ignores indentation changes.)

Updated

14 years ago
Flags: blocking1.7b?

Updated

14 years ago
Attachment #138278 - Flags: superreview?(dbaron)
Attachment #138278 - Flags: review?(prabhat.hegde)

Comment 17

14 years ago
Created attachment 141978 [details] [diff] [review]
Updated patch for 2004-02-22-trunk
Attachment #138278 - Attachment is obsolete: true

Comment 18

14 years ago
Created attachment 141979 [details] [diff] [review]
Updated patch for 2004-02-22-trunk (same as attachment 141978 [details] [diff] [review] but diff'ed with -u -w to ignore whitespace changes)

Comment 19

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

r=roland.mainz@nrubsig.org (original patch was from prabhat). The patch
compiles and fixes the problem... :)
Attachment #141978 - Flags: superreview?(bzbarsky)
Attachment #141978 - Flags: review+

Updated

14 years ago
Attachment #141978 - Flags: superreview?(bzbarsky) → superreview?(rbs)

Comment 20

14 years ago
(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.

Updated

14 years ago
Attachment #141978 - Flags: superreview?(rbs) → superreview?(roc)
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
Attachment #141978 - Flags: superreview?(roc) → superreview+
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)
Status: REOPENED → RESOLVED
Last Resolved: 15 years ago14 years ago
Resolution: --- → FIXED

Comment 23

14 years ago
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

14 years ago
(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

14 years ago
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

14 years ago
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

14 years ago
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") ... ;-(

Updated

14 years ago
Flags: blocking1.7b?

Updated

9 years ago
Component: Layout: CTL → Layout: Text
QA Contact: amyy → layout.fonts-and-text
You need to log in before you can comment on or make changes to this bug.