634406 - In Web Console, "Select All" unexpectedly results in multi-line input

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Frank Yan (:fryn)]

Steps to reproduce:
1. Open the Web Console.
2. Type something that has no autocompletion, e.g. ` or whatisthisidonteven.
3. Input field context menu -> "Select All". (On Mac, you can press Cmd+A. On Windows and Linux, Ctrl+A for Select All may still be broken due to bug 634403.)
4. Press the right arrow key to move the caret to the end of input.
5. Type a character.

Expected results:
The character is inserted immediately following the last character.

Actual results:
A new line has been inserted, and the character inserted is the first character on the second line.

Comment 2

[Frank Yan (:fryn)]

How is this a duplicate of bug 623749 ?

Comment 3

[David Dahl :ddahl]

(In reply to comment #2)
> How is this a duplicate of bug 623749 ?

oops. My dupe keybindings bug instincts are way to hair-triggered:)

Comment 4

[Panos Astithas (he/him) [:past] (please ni?)]

I get lots of these assertions when I type the final character after the right arrow:

###!!! ASSERTION: We should have one text node and one mozBR at most: 'length <= 2', file /Users/past/src/devtools/layout/forms/nsTextControlFrame.cpp, line 1021

The value in the input field is always correct AFAICT, it seems that the resizing process here somehow gets confused:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/console/hudservice/HUDService.jsm#4526

CCing Ehsan, since the only lead I found via Google for this assertion is bug 240933 (Plaintext editor should stop using <br> all over). Ehsan, do you happen to know why this assertion would get triggered in this case?

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

I observed the same behavior in the Scratchpad window, too. I couldn't find any other multi-line input fields to test this, but it does smell like a platform bug.

Comment 6

[Rob Campbell [:rc] (:robcee)]

sonofagun. It appears you are correct, sir!

=> core::widgetry

Comment 7

[Rob Campbell [:rc] (:robcee)]

make that ::editor.

Comment 8

[Boris Zbarsky [:bzbarsky]]

So this is curious.  From the editor's point of view, this is just an HTML <textarea> tag; that's what's sitting in the DOM.  I can't reproduce the problem with a normal textarea in an HTML document, though.

So what is being done differently in this case?  In particular, what's different about the behavior of cmd_SelectAll here?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Oh, and the height styling from comment 4 is _very_ unlikely to matter here.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Is http://mxr.mozilla.org/mozilla-central/source/toolkit/components/console/hudservice/HUDService.jsm#4962 interfering here? (custom Ctrl/Cmd+A behavior). Comment 0 mentions the context menu "Select All", which wouldn't be affected by this, but maybe that specific result was tainted by having previously used the shortcut?

Comment 11

[(no longer active)]

(In reply to comment #10)
> Is
> http://mxr.mozilla.org/mozilla-central/source/toolkit/components/console/
> hudservice/HUDService.jsm#4962 interfering here? (custom Ctrl/Cmd+A
> behavior). Comment 0 mentions the context menu "Select All", which wouldn't
> be affected by this, but maybe that specific result was tainted by having
> previously used the shortcut?

No, because I managed to reproduce this without Cmd+A.

Comment 12

[Rob Campbell [:rc] (:robcee)]

Scratchpad STR:

1. Open Scratchpad (F4)
2. Select the textbox, type Ctrl/Cmd-A to Select All
3. Type "something" (clearing the text)
4. Type Cmd-A again followed by a right arrow (still on the single line)
5. type a letter

Expected Results
Letter should appear immediately after "something"

Actual:
Letter appears on following line.

Comment 13

[Rob Campbell [:rc] (:robcee)]

apparently this works (able to reproduce the failure) with the down arrow instead of the right arrow too.

Comment 14

[(no longer active)]

OK, this is totally a regression from bug 240933, and also happens in a normal textarea element.

Comment 15

[Rob Campbell [:rc] (:robcee)]

we definitely want a unittest for this.

I can probably write one for the scratchpad pretty easily.

Comment 16

[Rob Campbell [:rc] (:robcee)]

then again, it's also trivial to add one for data:text/html,<textarea/> too.

Comment 17

[(no longer active)]

Attachment: Patch (v1)

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 534881 [details] [diff] [review]
Patch (v1)

Review of attachment 534881 [details] [diff] [review]:
-----------------------------------------------------------------

Comment 19

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/c4973e32a099

Comment 20

[(no longer active)]

(In reply to comment #15)
> we definitely want a unittest for this.
> 
> I can probably write one for the scratchpad pretty easily.

I don't think that would be necessary, but I won't stop you if you want to write that test.  ;-)

Comment 21

[Rob Campbell [:rc] (:robcee)]

(In reply to comment #20)
> (In reply to comment #15)
> > we definitely want a unittest for this.
> > 
> > I can probably write one for the scratchpad pretty easily.
> 
> I don't think that would be necessary, but I won't stop you if you want to
> write that test.  ;-)

I think the moment's passed.

But this bug is fixed in today's nightly! huzzah! "20110526030623"