Closed Bug 125161 Opened 23 years ago Closed 22 years ago

Enter key causes two line breaks in textarea

Categories

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

Product:
Core
Component:
DOM: Editor
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: skamio, Assigned: mozeditor)

Details

(Whiteboard: EDITORBASE-; fixinhand;)

Attachments

(1 file, 4 obsolete files)

patch to nsTextEditRules.cpp
1.30 KB, patch
glazou
: review+
kinmoz
: superreview+
Details | Diff | Splinter Review 
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.
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 ?
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
Attached patch a patch but revive Bug 98018 (obsolete) — — Splinter Review 
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.
Attached patch a patch but revive Bug 98018 (obsolete) — — Splinter Review 
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.
(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.

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
looks like a dupe of bug #102819
There's some similar stuff in bug 124733 also.
jfrancis said he'll fix this.
Assignee: kin → jfrancis
Status: ASSIGNED → NEW
OS: Linux → All
Hardware: PC → All
Target Milestone: mozilla0.9.9 → ---
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
nominating nsbeta1 since this bug is visible in text widgets.
Whiteboard: EDITORBASE 0.5 days → EDITORBASE 0.5 days nsbeta1
Keywords: nsbeta1-
Whiteboard: EDITORBASE 0.5 days nsbeta1 → EDITORBASE- 0.5 days
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
Attached patch proposed patch (obsolete) — — Splinter Review 
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
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
ugh.  been working on other stuff too long.  marking M1 to put on my current to-
do list.
Target Milestone: mozilla1.2alpha → M1
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
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 on attachment 98309 [details] [diff] [review]
patch to nsTextEditRules.cpp

sr=kin@netscape.com
Attachment #98309 - Flags: superreview+
Whiteboard: EDITORBASE-; fixinhand; need r=,sr= → EDITORBASE-; fixinhand;
fix landed on trunk
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
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.

Attachment

General

Creator:
Created:
Updated:
Size: