125161 - Enter key causes two line breaks in textarea

Status: VERIFIED FIXED

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

Description

[Shotaro Kamio]

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.

Comment 1

[Shotaro Kamio]

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

[kinmoz]

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.

Comment 3

[Shotaro Kamio]

Attachment: 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.

Comment 4

[Shotaro Kamio]

Attachment: 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.

Comment 5

[Shotaro Kamio]

(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.

Comment 6

[Shotaro Kamio]

Attachment: 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.

Comment 7

[Andrew Schultz]

looks like a dupe of bug #102819

Comment 8

[Ere Maijala (slow)]

There's some similar stuff in bug 124733 also.

Comment 9

[kinmoz]

jfrancis said he'll fix this.

Comment 10

[Joe Francis]

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.

Comment 11

[Joe Francis]

nominating nsbeta1 since this bug is visible in text widgets.

Comment 12

[Joe Francis]

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!  :-)

Comment 13

[Kevin McCluskey (gone)]

Bulk moving all nsbeta1 nominations to future-P1. If they are approved
(nsbeta1+) they will be moved to mozilla1.0

Comment 14

[Shotaro Kamio]

Attachment: proposed patch

This is a small change to the patch of Bug 132133.
This adds a mozBR if the last node is not mozBR.

Comment 15

[Charles Manske]

Joe: is "proposed patch" a good idea?
In "Steps to reproduce", note that this is for TEXTAREA, not regular HTML
Composer page.

Comment 16

[Joe Francis]

ugh.  been working on other stuff too long.  marking M1 to put on my current to-
do list.

Comment 17

[Joe Francis]

Attachment: patch to nsTextEditRules.cpp

This patch prevents forward deletion of trailing br in a textwidget, which is
the real problem in this bug.

Comment 18

[Joe Francis]

ready for reviews

Comment 19

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

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

r=glazman

Comment 20

[kinmoz]

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

sr=[e-mail]

Comment 21

[Joe Francis]

fix landed on trunk

Comment 22

[rubydoo123]

gratuitous verify, then has been fixed for some time now.