125766 - textarea: Select All + right arrow + Enter produces fake newlines

Status: RESOLVED INCOMPLETE

(Core :: DOM: Editor, defect)

Description

[Jørn Støylen]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Macintosh; U; PPC; en-US; rv:0.9.8+) Gecko/20020212
BuildID:    2002021208



Reproducible: Always
Steps to Reproduce:
1. Type a few lines of text in a textarea.
2. Select All (Cmd-A or menu, doesn't matter)
3. Press right arrow to move caret to end of text.
4. Press Enter.

Actual Results:  Pressing Enter produces two newlines.

There are a few exceptions:

1. Type a few characters.
2. Select all.
3. Right arrow.
4. Enter.

Does not produce two newlines. But instead try:

1. Type a few characters, then Enter, then Backspace (or Left arrow, Del instead
of Backspace).
Continue from point 2 above.

This produces 2 newlines.

It seems like this bug sometimes really does generate two newlines, and
sometimes only makes it look like there are two. I haven't been able to
determine why this happens or not, though.
Test case: This paragraph is made by utilizing this bug, and should have an
empty line before it.
Another test case: This paragraph, too.A third test case: Generated two newlines
with the bug after previous paragraph, deleted one of them and typed this paragraph.

Fourth test case: This paragraph should have two empty lines before it (one
produced by bug, one made with Enter).

Fifth test case: This paragraph should have three lines before it, all three
made by using the bug twice.

I'll let the above test results speak for themselves. :-)

Comment 1

[Boris Zbarsky [:bzbarsky]]

editor.  Seeing this on linux too....

Comment 2

[John Levon]

dup of bug 102819 ?

Comment 3

[Shotaro Kamio]

I confirmed on 0.9.8/Linux and 2002021621/Linux.
marked as NEW.

I think the reason of this bug is same as one of Bug 88024.
We must prevent the caret from going beyond the last BR node.

Comment 4

[Hixie (not reading bugmail)]

This is one of the most annoying bugs in mail compose right now. You end up
playing games trying to work out exactly how to get exactly one blank line
(delete sometimes goes up one line, backspace sometimes inserts a new line,
enter sometimes goes up a line...), then when you eventually have the editor
convinced that you've deleted the right number of invisible blank lines, you
press a character and a new blank line sometimes magically appears.

Comment 5

[Mike Pinkerton (not reading bugmail)]

not sure why this is targeted at moz1.2. it makes our editor a joke. i see it
with every email i reply to. every single one.

Comment 6

[kinmoz]

Shotaro is correct. This is a selection navigation problem, so I'm giving it to
mjudge and clearing the milestone, perhaps he has some cycles to look into this.

hixie and pinkerton, file bugs regarding the behaviors you are talking about.
This particular bug is specific to textareas, so lets not morph this into a mail
compose bug.

Comment 7

[Hixie (not reading bugmail)]

I see this mainly in Bugzilla's <textarea>s, but the same bug appears in Mail's
plain text editor. Are you saying that is a different codebase?

Comment 8

[kinmoz]

I just tried the same steps to reproduce this bug in Composer, Composer's
PlainText Editor, HTML Mail Compose, and Plaintext Mail Compose, and I *cannot*
reproduce the problem in any of them.

Text widgets have a different selection controller (which also controls caret
navigation) and different edit rules.

Comment 9

[Syd Logan]

EDITORBASE-, but nominating topembed since this happens in text areas.

Comment 10

[Marek Z. Jeziorek]

Marking it topembed-, embed

Comment 11

[mjudge]

marking 1.1 alpha

Comment 12

[Samir Gehani]

adt: nsbeta1-

Comment 13

[Prognathous]

asfs fsdlj fsdlkj

Comment 14

[Uri Bernstein]

Note that in step 4 you can type any character (instead of hitting Enter), and the typed character will appear on a new line.

Moving to Selection and assigning to default owner.

Comment 15

[Uri Bernstein]

*** Bug 342684 has been marked as a duplicate of this bug. ***

Comment 16

[(no longer active)]

This bug doesn't have anything to do with bug 240933...

Comment 17

[Mike Schroepfer]

I'm able to reproduce this in Firefox 6 but not in the Firefox 8 nightlies.  Has this been resolved recently?

Comment 18

[(no longer active)]

(In reply to Mike Schroepfer from comment #17)
> I'm able to reproduce this in Firefox 6 but not in the Firefox 8 nightlies. 
> Has this been resolved recently?

Well, not intentionally, but yes, I can't reproduce it on trunk either.  :-)  Worth adding a test to make sure that we won't regress it in the future.

Fabien, would you mind writing a test for this?  Thanks!

Comment 19

[Fabien Cazenave [:kaze]]

Attachment: patch proposal

Here’s a couple of tests for this bug and for bug 378194. I’m pulling an old branch atm to make sure that this test fails on Firefox 6.

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&tree=Try&rev=60791ed11975

Comment 20

[(no longer active)]

Comment on attachment 559468 [details] [diff] [review]
patch proposal

># HG changeset patch
># Parent 72938723787d1872769e511667c21c8e73293698
># User Fabien Cazenave <[e-mail]>
>Bug 125766 - textarea: Select All + right arrow + Enter produces fake newlines
>
>This bug has been solved by mistake, here are some tests to make sure we don't
>reopen it in the future.

I wouldn't say that this is fixed "by mistake", it's just been fixed because we've fixed some other bug somewhere else in the code.  ;-)

>+function runTests() {
>+  var editor = document.getElementById("content");
>+  var sel = window.getSelection();

You don't want to use window's selection here, it cannot operate on selections inside text controls, since those have their own selection controller.

>+  // bug 125766: right arrow + text should not create any new line
>+  editor.value = original;
>+  editor.focus();
>+  sel.selectAllChildren(editor);

Instead of this, you want to simulate Accel+A.

>+  synthesizeKey("VK_RIGHT", {});
>+  synthesizeKey("x", {});
>+  is(editor.value, original + "x", "unwanted characters have been added in <textarea>.");
>+
>+  // bug 125766: same test, but typing "foo bar" manually + Return/BS
>+  editor.value = "";
>+  editor.focus();
>+  synthesizeKey("f", {});
>+  synthesizeKey("o", {});
>+  synthesizeKey("o", {});
>+  synthesizeKey(" ", {});
>+  synthesizeKey("b", {});
>+  synthesizeKey("a", {});
>+  synthesizeKey("r", {});
>+  sel.selectAllChildren(editor);

Ditto.

>+  synthesizeKey("VK_RETURN", {});
>+  synthesizeKey("VK_BACK_SPACE", {});
>+  synthesizeKey("x", {});
>+  is(editor.value, original + "x", "unwanted characters have been added in <textarea>.");

Instead of this, let's select all, press right, and then press enter, and then test the value to make sure that it only has a single trailing line break.

Comment 21

[Fabien Cazenave [:kaze]]

Hmm, <textarea>.value strips all CR/LF so I doubt we can test this with a mochitest.

I could do a reftest but I haven’t found any way to use enhanced privileges (required for synthesizeKey) in reftests. Is there a workaround?

Comment 22

[Fabien Cazenave [:kaze]]

Trying to explain more precisely…

This bug can be reproduced on Firefox 4 and older. We can see these unwanted newlines in the <textarea>, but these unwanted newlines don’t appear at all in <textarea>.value.
Or course, if [Return] is pressed in a textarea, <textarea>.value will show a line feed (\n) but not the unwanted newlines. That’s why a Mochitest doesn’t help here.

Comment 23

[(no longer active)]

You can use the snapshotWindow and compareSnapshot APIs here <http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/WindowSnapshot.js> to capture screenshots of the test page and compare them against each other.  There are tests in the editor/ directory which do that, you can use them as examples.

Comment 24

[Fabien Cazenave [:kaze]]

Attachment: patch proposal

thanks for the pointers!
Here’s a better test. It fails with Firefox 4 but not with the latest trunk.

Comment 25

[Fabien Cazenave [:kaze]]

Attachment: patch proposal

(In reply to Ehsan Akhgari [:ehsan] from comment #20)
> Instead of this, let's select all, press right, and then press enter, and
> then test the value to make sure that it only has a single trailing line
> break.

Forgot about that. This patch does this for this bug and for bug 378194.

Comment 26

[(no longer active)]

(In reply to Fabien Cazenave (:kaze) from comment #25)
> Created attachment 563171 [details] [diff] [review] [diff] [details] [review]
> patch proposal
> 
> (In reply to Ehsan Akhgari [:ehsan] from comment #20)
> > Instead of this, let's select all, press right, and then press enter, and
> > then test the value to make sure that it only has a single trailing line
> > break.
> 
> Forgot about that. This patch does this for this bug and for bug 378194.

What do you mean?

Comment 27

[Fabien Cazenave [:kaze]]

I didn’t take that point in comment #20 into account for attachment 563161 [details] [diff] [review].
Your comment should be addressed by my latest patch. Sorry for the confusion.

Comment 28

[Andrei Purice]

Hey Fabien,
Can you still reproduce this or should we close it?

Comment 29

[Andrei Purice]

Marking this as Resolved > Incomplete since the last activity on this issue was 10 years ago and it might not be relevant anymore.
Feel free to re-open if the issue is still reproducible on your end in the latest FF versions.