Closed Bug 14588 Opened 21 years ago Closed 20 years ago

Ctrl+right arrow skips over nbsp (nbsp should be word break)

Categories

(Core :: DOM: Editor, defect, P3)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: sujay, Assigned: shanjian)

Details

(Whiteboard: [nsbeta3+]fix checked in.)

Attachments

(3 files)

using 9/22 build of apprunner

1) launch apprunner
2) launch editor
3) place cursor at beginning of 2nd line("Here is an acute entity..")
4) start doing Ctrl+right arrow key to move cursor one word at a time

it does seem to work in the beginning, but once you get to this
word: "This sentence has two &nbsp" you'll notice the cursor
jumps to the end of that line, instead of going to each word
in that sentence..

all platforms.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → INVALID
this is expected behavior since there aren't any spaces (only nbsp's and
characters) in that sentence.  I tried to test 4.x but it eats/mangles the nbsp's
so it can't be used for comparison.

marking this invalid if/until we decide to change the behavior.
Simon? Charley?
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
The key and double-click handling should treat nbsps as white space. So sujay was
correct to file this bug.
ok, I agree.  We should start to think about whether we also want a hyphen to be
a breaking character or not.  (I wonder if there are any i18n issues here? ;-) )

note: this bug isn't just for right arrow but also when using left arrow.
I remember hearing that I18N folks were responsible for deciding what is a
"whitespace" character, and thus the word separator for caret navigation,
and they want nbsp to NOT be whitespace. IMHO, this is wrong.
Assignee: mjudge → ftang
Status: REOPENED → NEW
1. It is true that i18n should own the line / word break and we have an api for
deciding line/word break.
2. It is not ture that we want to decie what is a whitespace.

I will just reassign this bug to myself and fix it right away. Let me make one
thing clear, you want to treat nbsp as word seperating boundary, right ?
Status: NEW → ASSIGNED
give me couple minutes, and I should fix it.
Frank, yes I think nbps's should be breaking points.  What about dashes?

Frank--when you finish you part of the bug, please reassign to mjudge for him to
fix the editor hooks to call your api.
We already hookup the line/word break class for a long long long time. The
problme actually is not inside my word breaking class, but nsTextTransformer.cpp
itself. Why don't I just fix it for you. With mjudge's code review, of course.
adding Mike to cc list.  Mike, are you ok with Frank's plan?
hum... thing are not that easy. What should we do with two nbsp together there?
How important is this bug ? It looks like it is not easy as I expected. The
white space compression code and the mixed with linebreaking code in
nsTextTransformer + the way nsTextTransformer implement (by using both 1byte
buffer and 2 byte buffer) make this difficult than I thought. Not a one liner
for me....
Target Milestone: M14
Mark it M14
Blocks: 19265
No longer blocks: 19265
Not that anyone likely wants this bug complicated by any more issues,
but now is probably a better time than after a fix is made:

There are several characters in the "Special Characters" section of the
"Character Entities References" chapter of the HTML 4.0 spec
<URL:http://www.w3.org/TR/REC-html40/sgml/entities.html#h-24.4.1>
that almost certainly should be treated as whitespace for the purposes of
line-breaking, and there may be some others.

Prime candidates:
<!ENTITY ensp    CDATA "&#8194;" -- en space, U+2002 ISOpub -->
<!ENTITY emsp    CDATA "&#8195;" -- em space, U+2003 ISOpub -->
<!ENTITY thinsp  CDATA "&#8201;" -- thin space, U+2009 ISOpub -->
<!ENTITY zwnj    CDATA "&#8204;" -- zero width non-joiner, U+200C NEW RFC 2070
-->

Possibles, if hyphen is to be a breaking char.
<!ENTITY ndash   CDATA "&#8211;" -- en dash, U+2013 ISOpub -->
<!ENTITY mdash   CDATA "&#8212;" -- em dash, U+2014 ISOpub -->
I don't think this should be beta. Move it to M17
Target Milestone: M14 → M17
updating summary to be more accurate; cc akkana since some of her work will rely 
on this functionality
Summary: Ctrl+right arrow doesn't jumps to end of line → Ctrl+right arrow skips over nbsp (nbsp should be word break)
move to M18
Target Milestone: M17 → M18
reassign to shanjian
Assignee: ftang → shanjian
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Added ftang to cC list.

Frank, could you review the fix in your convenience?

nsbeta3+ per i18n bug review meeting.
sorry, I have not review the code yet.
Keywords: correctness, nsbeta3
Whiteboard: [nsbeta3+]
fix has been checked in.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago20 years ago
Resolution: --- → FIXED
Reopened: we had to back out the changes to nsTextTransformer.cpp due to 
multiple regressions. I'm assuming that this bug will have to be re-addressed 
now, so reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
the new fix has 2 improvement over previous one, which will not 
cause regressions found in previous fix.
1, nbsp is only treated as space character (isWhitespace = PR_TRUE;)
   when it is not for line break. 
2, If nbsp is the fist character, it will not be broken immediately,
   this make sure we advance at least one word ahead, thus fixed the 
   possible hanging.
Whiteboard: [nsbeta3+] → [nsbeta3+]wait for code review
Status: REOPENED → ASSIGNED
I think your latest fix is still too risky. Is that possible you can have two
version of
 nsTextTransformer::ScanNormalAsciiText_F(PRInt32* aWordLen,PRBool*
aWasTransformed)

Say ScanNormalAsciiText_F and ScanNormalAsciiText_F_ForWordBreak and have a if
statement in the
GetNextWord to switch between them. Keep the ScanNormalAsciiText_F the same as
today and put your fix into the new ScanNormalAsciiText_F_ForWordBreak routine?
In this way, we can 100% sure your change wont't change line break in layout.
the fix was modified according to ftang's suggestion. New fix only apply 
to wordjump.

fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
Whiteboard: [nsbeta3+]wait for code review → [nsbeta3+]fix checked in.
verified in 8/30 build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.