Enter key causes two line breaks in textarea

VERIFIED FIXED in M1

Status

()

P1
normal
VERIFIED FIXED
17 years ago
16 years ago

People

(Reporter: skamio, Assigned: mozeditor)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: EDITORBASE-; fixinhand;)

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

17 years ago
Steps to reproduce
1. Type "foo"
2. Hit "Enter" key
3. Hit "DEL" key
4. Hit "Enter" key

Then, you will see two line breaks made.
And it is strange that "DEL" key deletes an empty line where the caret is.

I confirmed on Mozilla0.9.8/Linux and 2002021121/Linux.
(Reporter)

Comment 1

17 years ago
I could find a suspicious point on source code
at line 430 in nsTextEditRules::DidInsertBreak() on nsTextEditRules.cpp
http://lxr.mozilla.org/seamonkey/source/editor/libeditor/text/nsTextEditRules.cpp#430

mEditor->GetStartNodeAndOffset(...) is used there.
When I hit "Enter" on Step 4 of the comment above,
 it returns the variable "selOffset" = 3.
On that time, textarea has one text node and one BR node.
So "selOffset" must be 1 or 2(is it valid?).
3 is out of range.
Why ?

Comment 2

17 years ago
I think part of the problem here is that we shouldn't be allowing people to
forward delete a <BR> if it's the last thing the content tree for the plaintext
widget (eEditorWidgetMask) case. Perhaps we need to add code to handle this in
nsTextEditRules::WillDeleteSelection().

Cc jfrancis.
Status: NEW → ASSIGNED
Priority: -- → P3
Whiteboard: EDITORBASE 0.5 days
Target Milestone: --- → mozilla0.9.9
(Reporter)

Comment 3

17 years ago
Created attachment 69417 [details] [diff] [review]
a patch but revive Bug 98018

This patch seems to fix this bug.
But it revives Bug 98018.

However I don't know much,
 it seems to me that true reason of Bug 98018 is in the other point.
(Reporter)

Comment 4

17 years ago
Created attachment 69418 [details] [diff] [review]
a patch but revive Bug 98018

This patch seems to fix this bug.
But it revives Bug 98018.

However I don't know much,
 it seems to me that true reason of Bug 98018 is in the other point.
(Reporter)

Comment 5

17 years ago
(comments posted twice, sorry.)

I wrote a patch for debug purposes.
This will point out what I said above.
If there is my misunderstanding of codes, please say so.
(Reporter)

Comment 6

17 years ago
Created attachment 69616 [details] [diff] [review]
a patch for debug which revives Bug 98018 and adds some debug codes

Try this patch.
And try to reproduce Bug 98018.

Then, you'll see debug messages like "IsRootNode()!".
Why the node is RootNode? It must be a text node.
Where is the caret actually ?
That is a strange behavior.
Attachment #69417 - Attachment is obsolete: true
Attachment #69418 - Attachment is obsolete: true

Comment 7

17 years ago
looks like a dupe of bug #102819

Comment 8

17 years ago
There's some similar stuff in bug 124733 also.

Comment 9

17 years ago
jfrancis said he'll fix this.
Assignee: kin → jfrancis
Status: ASSIGNED → NEW

Updated

17 years ago
OS: Linux → All
Hardware: PC → All
Target Milestone: mozilla0.9.9 → ---
(Assignee)

Comment 10

17 years ago
Currently forward delete will delete a blank line you are on.  This is usually 
correct.  For instance, if there is anything *after* that blank line, you expect 
the line to be deleted.

The exception to this is when there is nothing after it.  So I need to test for 
that case.

Another thin I should check is that if you have a blank line at the end of a 
block element, and hit forward delete, I should pull in the content after the 
block in addition to deleting that blank line.
Status: NEW → ASSIGNED
(Assignee)

Comment 11

17 years ago
nominating nsbeta1 since this bug is visible in text widgets.
Whiteboard: EDITORBASE 0.5 days → EDITORBASE 0.5 days nsbeta1

Updated

17 years ago
Keywords: nsbeta1-
Whiteboard: EDITORBASE 0.5 days nsbeta1 → EDITORBASE- 0.5 days
(Assignee)

Comment 12

17 years ago
pushing off.  I'm hesitant to go further than 1.0 or lower than P2 because I 
disagree with the EB- call.  But if Kevin or Syd want to overrule me go ahead.  
That's why they pay you the big bucks!  :-)
Priority: P3 → P2
Target Milestone: --- → mozilla1.0
Bulk moving all nsbeta1 nominations to future-P1. If they are approved
(nsbeta1+) they will be moved to mozilla1.0
Priority: P2 → P1
Target Milestone: mozilla1.0 → Future
(Reporter)

Comment 14

17 years ago
Created attachment 78320 [details] [diff] [review]
proposed patch

This is a small change to the patch of Bug 132133.
This adds a mozBR if the last node is not mozBR.
Attachment #69616 - Attachment is obsolete: true

Comment 15

16 years ago
Joe: is "proposed patch" a good idea?
In "Steps to reproduce", note that this is for TEXTAREA, not regular HTML
Composer page.
Keywords: nsbeta1- → nsbeta1
Target Milestone: Future → mozilla1.2alpha
(Assignee)

Comment 16

16 years ago
ugh.  been working on other stuff too long.  marking M1 to put on my current to-
do list.
Target Milestone: mozilla1.2alpha → M1
(Assignee)

Comment 17

16 years ago
Created attachment 98309 [details] [diff] [review]
patch to nsTextEditRules.cpp

This patch prevents forward deletion of trailing br in a textwidget, which is
the real problem in this bug.
Attachment #78320 - Attachment is obsolete: true
(Assignee)

Comment 18

16 years ago
ready for reviews
Whiteboard: EDITORBASE- 0.5 days → EDITORBASE-; fixinhand; need r=,sr=
Comment on attachment 98309 [details] [diff] [review]
patch to nsTextEditRules.cpp

r=glazman
Attachment #98309 - Flags: review+

Comment 20

16 years ago
Comment on attachment 98309 [details] [diff] [review]
patch to nsTextEditRules.cpp

sr=kin@netscape.com
Attachment #98309 - Flags: superreview+

Updated

16 years ago
Whiteboard: EDITORBASE-; fixinhand; need r=,sr= → EDITORBASE-; fixinhand;
(Assignee)

Comment 21

16 years ago
fix landed on trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 22

16 years ago
gratuitous verify, then has been fixed for some time now.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.