Too many stops for Ctrl-RightArrow ("a b" stop 3 times instead of 2)

RESOLVED FIXED

Status

()

Core
Editor
--
major
RESOLVED FIXED
16 years ago
11 years ago

People

(Reporter: Brent, Assigned: Vaclav Dvorak)

Tracking

({access})

Trunk
access
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.3 -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: EDITORBASE+)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

16 years ago
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0rc1) Gecko/20020417
BuildID:    2002041711

Ctrl-RightArrow stops before and after whitespace.  This happens anywhere text
can be entered:  in a textarea widget, on a single line of input, in mail, and
even in the URL input line at the top of the browser window.  Whichever position
is correct (before or after), stopping at both places is wrong.

Reproducible: Always
Steps to Reproduce:
In any text input, such as the URL address bar at the top of the browser window:
1.  Type "a b"
2.  Hit <Home> and <Ctrl-RightArrow>.  Caret moves between "a ".
3.  Hit <Ctrl-RightArrow>.  Caret moves between " b".

Actual Results:  Cursor did not advance past any non-whitespace.

Expected Results:  Either the first <Ctrl-RightArrow> should move the caret
between " b",
or the second <Ctrl-RightArrow> should move the caret after the "b".  It doesn't
matter which, as long as it is not BOTH.

See bug 108125 for description of what various editors do,
and bug 92850 for a slightly different problem.  After consideration, I thought
it best to create a new bug report rather than stretch bug 92850 or bug 108125
to cover this problem.

Tried to come up with the absolute simplest logic for Ctrl-RightArrow:

Boolean  is_whitespace(unsigned char  c) {
  Boolean  ws_table[256] = { T, F, F, F, F,  F, F, F, T, F,
                             T, F, T, T, F, /* 256 entries */ };
  return  ws_table[c];
}

if (! buffer_is_empty) {
  cu = is_whitespace(character at current position of caret);
  do {
    if (caret at end of text) break;
    move caret 1 position forward;
    pr = cu;
    cu = is_whitespace(character at current position of caret);
  } while (!pr || cu);
}

Comment 1

16 years ago
--> mjudge@netscape.com (caret/selection navigation)
Assignee: kin → mjudge
(Reporter)

Comment 2

16 years ago
2002041206 and 2002041711 (RC1) both on Windows, do not have this
Ctrl-RightArrow problem.  Maybe this is Linux only, but thought I had seen this
on a Windows version as well (0.9.9 maybe).

Comment 3

16 years ago
WFM win2000 2002062304.

reporter (Brent): can you reproduce this problem with a recent build of mozilla
(for example, 1.1alpha)? if so, please comment again with details (including
what proxy you are using). if not, please resolve this bug as WORKSFORME. thank you.
(Reporter)

Comment 4

16 years ago
Bug is present under Linux in 1.0 and 1.1a (build 2002061108)
It is present on every Linux system I have tried.  I tried
Slackware 8.0, Redhat 7.0, 7.2, and 7.3.  I tried the build
that came with Redhat 7.3 (2002040813) and 1.1a.

Proxy?  I don't know what you mean by proxy, so here's a
bunch of probably useless details:

I install by getting the tar.gz file (NOT the "sea" one), then
#cd /usr/lib
#rm -Rf mozilla/*
#gzip -d < ~/mozilla...tar.gz | tar x

What ldd says for 1.1a under Slackware 8.0:
	libgkgfx.so => /opt/gnome/lib/libgkgfx.so (0x40021000)
	libjsj.so => /opt/gnome/lib/libjsj.so (0x40053000)
	libmozjs.so => /opt/gnome/lib/libmozjs.so (0x4006d000)
	libxpcom.so => /opt/gnome/lib/libxpcom.so (0x400d6000)
	libplds4.so => /opt/gnome/lib/libplds4.so (0x401a6000)
	libplc4.so => /opt/gnome/lib/libplc4.so (0x401a9000)
	libnspr4.so => /opt/gnome/lib/libnspr4.so (0x401ae000)
	libpthread.so.0 => /lib/libpthread.so.0 (0x401d8000)
	libdl.so.2 => /lib/libdl.so.2 (0x401ee000)
	libgtk-1.2.so.0 => /opt/gnome/lib/libgtk-1.2.so.0 (0x401f2000)
	libgdk-1.2.so.0 => /opt/gnome/lib/libgdk-1.2.so.0 (0x40317000)
	libgmodule-1.2.so.0 => /opt/gnome/lib/libgmodule-1.2.so.0 (0x40349000)
	libglib-1.2.so.0 => /opt/gnome/lib/libglib-1.2.so.0 (0x4034c000)
	libXi.so.6 => /usr/X11R6/lib/libXi.so.6 (0x4036f000)
	libXext.so.6 => /usr/X11R6/lib/libXext.so.6 (0x40378000)
	libX11.so.6 => /usr/X11R6/lib/libX11.so.6 (0x40386000)
	libm.so.6 => /lib/libm.so.6 (0x4045f000)
	libc.so.6 => /lib/libc.so.6 (0x40481000)
	libstdc++-libc6.1-1.so.2 =>
/usr/i386-slackware-linux/lib/libstdc++-libc6.1-1.so.2 (0x40592000)
	libstdc++-libc6.2-2.so.3 => /usr/lib/libstdc++-libc6.2-2.so.3 (0x405d4000)
	/lib/ld-linux.so.2 => /lib/ld-linux.so.2 (0x40000000)
OS: All → Linux
see it too on Linux, 2002073022.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 6

15 years ago
I see this on Macintosh also so marking this as all/all even though Windows is
not effected.  I believe there is a pref set for linux/Mac users which is at
fault (windows users would see this also if they changed whatever the pref is).

Nominate for editorbase.
There is probably a duplicate bug on this issue.
OS: Linux → All
Hardware: PC → All
Summary: Too many stops for Ctrl-RightArrow → Too many stops for Ctrl-RightArrow ("a b" stop 3 times instead of 2)
Whiteboard: EDITORBASE

Comment 7

15 years ago
Adding EDITORBASE+, affects all text widgets, nominating nsbeta1
Keywords: nsbeta1
Whiteboard: EDITORBASE → EDITORBASE+
Root of the problem is
http://lxr.mozilla.org/seamonkey/search?string=eat_space_to_next

I did not make a patch to change that since (a) I don't really know if that pref
controls more than just double-clicking on a word and ctrol-arrow (b) what's
the common behavior on linux and mac for word selection.

Keywords: access

Updated

15 years ago
QA Contact: sujay → beppe
(Assignee)

Comment 9

15 years ago
Requesting blocking1.3 - this is annoying, and should be simple to fix, I suppose?
Flags: blocking1.3?
The default behavior in GNOME 2 text widgets seems to be that Ctrl-Rightarrow
stops before (i.e., without crossing) the space (and Ctrl-Leftarrow stops after
(again, without crossing) the space).  This is the behavior I'd expect.

Comment 11

15 years ago
Leaving EDITORBASE+ because the behavior is different when going left to right
vs. right to left. Seen on MacOS X

Comment 12

15 years ago
We should get this fixed. mjudge, can you take a look at this and let us know
what can be done? Thanks. 
Flags: blocking1.3? → blocking1.3+
Keywords: nsbeta1 → nsbeta1+

Comment 13

15 years ago
not a recent regression. pulling from 1.3 blocker list.
Flags: blocking1.3+ → blocking1.3-
*** Bug 187982 has been marked as a duplicate of this bug. ***

Comment 15

14 years ago
 this is still happening in the 1.5 release candidate.
I cannot type due to a repetitive stress injury and must use voice recognition
software instead, which means I also cannot use a mouse - so keyboard
equivalents program through my voice commands are everything to me. The mismatch
of left and right side movements that this bug causes drives me absolutely insane.

I don't normally beg, but in this case I'm heading for my knees...
(Assignee)

Updated

14 years ago
Flags: blocking1.6a?
(Assignee)

Comment 16

14 years ago
Created attachment 145923 [details] [diff] [review]
proposed patch

Here's a patch that fixes this. As far as I can tell, it works fine (unless I
screwed up the diffing). When testing, be aware of bug 237228 - you can't
change the eat_space pref in prefs.js or user.js unless you also apply my patch
in that bug (which you currently can't because it conflicts with this one).

The patch streamlines the code quite a bit, because there was a section of
redundant code (if (cond) do_code; else do_exact_same_code;), and conditions
that were always true. Still, even after this patch, the code seems kinda
messy, and I've found cases with caret browsing where it doesn't work as it
should (even without the patch). I'll file a bug on those.
(Assignee)

Updated

14 years ago
Assignee: mjudge → vdvo
Status: NEW → ASSIGNED
(Assignee)

Comment 17

14 years ago
Created attachment 145924 [details] [diff] [review]
same patch without whitespace changes - a bit more readable
(Assignee)

Comment 18

14 years ago
Comment on attachment 145923 [details] [diff] [review]
proposed patch

Hmm, any chance it could still go into 1.7?
Attachment #145923 - Flags: superreview?(jst)
Attachment #145923 - Flags: review?(roc)
Comment on attachment 145923 [details] [diff] [review]
proposed patch

I don't *really* understand this code, but I don't think anyone does right now.
You're certainly simpifying it, which is great. And the changes look reasonable
to me.
Attachment #145923 - Flags: review?(roc) → review+
+            if ((sWordSelectEatSpaceAfter && (isWhitespace || !aPos->mEatingWS))
+                || (!sWordSelectEatSpaceAfter && (!isWhitespace ||
!aPos->mEatingWS))) {

This can be simplified to

  if ((sWordSelectEatSpaceAfter ? isWhitespace : !isWhitespace)
      || !aPos->mEatingWS) {

Which I think is clearer. Similarly,

+                if ((isWhitespace && sWordSelectEatSpaceAfter) ||
(!isWhitespace && !sWordSelectEatSpaceAfter))

can be simplified to

  if (sWordSelectEatSpaceAfter ? isWhitespace : !isWhitespace)

In both cases, "sWordSelectEatSpaceAfter ? isWhitespace : !isWhitespace" could
be simplified to "sWordSelectEatSpaceAfter == isWhitespace", but I don't think
that's as clear.
I don't think this is worth putting in 1.7. For one thing, this code is ugly and
not very well understood so there's some risk of regressions. And the bug is
annoying but not critical.
Comment on attachment 145923 [details] [diff] [review]
proposed patch

+	       aPos->mEatingWS = ! sWordSelectEatSpaceAfter;

Loose the space after ! for consistency with the surrounding code.

sr=jst
Attachment #145923 - Flags: superreview?(jst) → superreview+
(Assignee)

Comment 23

14 years ago
Created attachment 146142 [details] [diff] [review]
new patch, ready for checkin

Here's the patch with the requested changes. Thanks for the reviews. Could one
of you check this in for me, please?
Attachment #145923 - Attachment is obsolete: true
(Assignee)

Comment 24

14 years ago
Could someone spare a few minutes to check the patch in, please? Thanks!

Comment 25

14 years ago
for the record, Neil checked the patch in at 2004-04-19 08:33. someone affected
by this should check if it works and if so mark this bug fixed.
Blocks: 236172
(Assignee)

Comment 26

14 years ago
Verified with latest nightly. Thanks for the checkin, Neil.
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED

Comment 27

14 years ago
The fix is not yet perfect.

Finally, in 1.8.a the bug is fixed, but the fix is not yet perfect:
Ctrl-RightArrow now stops only at the END of words. (differently to other text
editors, in which BOTH Ctrl-RightArrow and Ctrl-LeftArrow stop at the BEGINNING
of words - I think that is usual, expected behavior).  

(In 1.8.a (just like in previos verions), Ctrl-LeftArrow stops correctly at the
beginnign of words)

Please can this be also fixed (or is my idea wrong) ?

Comment 28

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

Comment 29

14 years ago
(In reply to comment #27)
> Finally, in 1.8.a the bug is fixed, but the fix is not yet perfect:
> Ctrl-RightArrow now stops only at the END of words. (differently to other text
> editors, in which BOTH Ctrl-RightArrow and Ctrl-LeftArrow stop at the BEGINNING
> of words - I think that is usual, expected behavior).  

First I would like to thank Vaclav and the others for their work on fixing this bug.

In replying to the comment above, I am not sure which platform you are talking
about, but I would say this fix works correctly on the Macintosh. Option-left
stops at the beginning of words and option-right stops at the end of words. And
informal check of some text editors (AppleWorks, TextEdit, BB Edit) shows that
their handling of white space and punctuation varies, so I think Mozilla works
just fine.

Comment 30

14 years ago
Also, I would like to thank Vaclav and the others for their work. The situation
is now much better. Basically, the bug (double-stopping) was fixed. And it is
another question where we want the cursor to be stoping (either beginning or end
of words). 

In my previous post, I am talking about behavior on Linux. On Windows it stops
at the begining in both directions for a long time.

I only thought that stoping at the begining in both directions is a "standard"
that we should follow (Mozilla on Windows also stops at at the begining in both
directions). here are some more progs that I tested and that follow this "standard":

On Windows:
Mozilla 1.7.0, Thunderbird 0.7.1, Firefox 0.9, M$ Word 97, Notepad, Wordpad
(both from Winodws 98 SE), Edipad Classic

On DOS:
Norton Commander, DOS Navigator, Volkov Commander

On Linux:
Midnight Commander, OpenOfice 1.1 Writer, Kate, Kwrite, KWord, KMail, Quanta
Plus (this may be a feature of KDE-libs, so probably any KDE app behaves this way)

HOWEVER, on linux I found apps that behave differently:
GEdit, GNumeric - Ctrl+Right stops at the end of words (again, this may be a
feature of an underlying Gnome or GTK lib)

Sorry I have no MacOS to test there.

I don't want to discuss stoping before/after dots, commas, dashes, e.t.c, where
probably every app has its own behavior.

I thought, that if majority applications stops at beginnig of words in both
directions, and Mozilla also does on Windows, it should do it that way also on
Linux. 

Vaclav: I would like to ask you, if you still have some time, and consider my
thoughts right (and, maybe, if others would agree too), to change the Ctrl+Right
code to stop at beginning of words. (Because You have alredy studied that part
of Mozilla code, maybe for you this would be an easy fix.)

However, I can also live with it like it is coded now, but I think many other
users would like it that way I told too.

   Thank You again

Comment 31

13 years ago
I think Mozilla's behaviour now is as same as gedit or emacs.
It's good on linux/*nix.

But this patch may cause a problem,
Go to http://www.mozilla.org/start/1.7/,
Find "Getting Started", move caret into "Getting", press ctrl+right, it will go 
to end of "Getting". Press ctrl+right again, it will go to begin of the next 
line instead of end of "Started".

Comment 32

13 years ago
reopen this bug and paste patch here? or file a new bug "Too few stops for Ctrl-
RightArrow"? :-)

Comment 33

13 years ago
I'm not sure what you mean, but I tried some stuff out, and it looks as if
Ctrl+Right ignores spaces after closing tags, e.g.
<b>bold</b> <i>italic</i> <s>strikeout</s> <u>underline</u> (deprecated)
Place the cursor in deprecated and press Ctrl+Left a few times then Ctrl+Right!

Comment 34

13 years ago
Yes, your testcase is the same problem.

Comment 35

13 years ago
Is it reasonable to stop searching at end of TextFrame?
Press Ctrl+Right again it will go to next TextFrame.

@@ -4221,7 +4221,10 @@ nsTextFrame::PeekOffset(nsPresContext* a
                         ? mContentOffset + mContentLength : -1;
 #endif // IBMBIDI
               }
-              keepSearching = (mContentOffset + mContentLength) <=
aPos->mContentOffset;
+              if (sWordSelectEatSpaceAfter)
+                keepSearching = (mContentOffset + mContentLength) <=
aPos->mContentOffset;
+              else
+                keepSearching = (mContentOffset + mContentLength) <
aPos->mContentOffset;
               if (!keepSearching)
                 found = PR_TRUE;
             }

Comment 36

13 years ago
Created attachment 156330 [details]
neil's testcase

Comment 37

13 years ago
bug 256252 filed
*** Bug 298251 has been marked as a duplicate of this bug. ***

Updated

11 years ago
Blocks: 181926
You need to log in before you can comment on or make changes to this bug.