Last Comment Bug 634406 - In Web Console, "Select All" unexpectedly results in multi-line input
: In Web Console, "Select All" unexpectedly results in multi-line input
Status: VERIFIED FIXED
[console-1][scratchpad]
: regression
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla7
Assigned To: :Ehsan Akhgari (busy, don't ask for review please)
:
Mentors:
Depends on:
Blocks: 240933
  Show dependency treegraph
 
Reported: 2011-02-15 14:20 PST by Frank Yan (:fryn)
Modified: 2011-05-26 10:31 PDT (History)
9 users (show)
ehsan: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (v1) (4.34 KB, patch)
2011-05-24 14:14 PDT, :Ehsan Akhgari (busy, don't ask for review please)
roc: review+
Details | Diff | Review

Description Frank Yan (:fryn) 2011-02-15 14:20:33 PST
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 1 David Dahl :ddahl 2011-02-15 14:24:40 PST

*** This bug has been marked as a duplicate of bug 623749 ***
Comment 2 Frank Yan (:fryn) 2011-02-15 14:35:42 PST
How is this a duplicate of bug 623749 ?
Comment 3 David Dahl :ddahl 2011-02-15 14:59:48 PST
(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 [:past] 2011-05-16 08:43:23 PDT
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 [:past] 2011-05-17 02:08:35 PDT
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) 2011-05-17 07:04:08 PDT
sonofagun. It appears you are correct, sir!

=> core::widgetry
Comment 7 Rob Campbell [:rc] (:robcee) 2011-05-17 07:04:26 PDT
make that ::editor.
Comment 8 Boris Zbarsky [:bz] 2011-05-18 10:16:02 PDT
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 [:bz] 2011-05-18 10:18:31 PDT
Oh, and the height styling from comment 4 is _very_ unlikely to matter here.
Comment 10 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-05-24 12:25:50 PDT
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 :Ehsan Akhgari (busy, don't ask for review please) 2011-05-24 12:29:58 PDT
(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) 2011-05-24 12:35:05 PDT
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) 2011-05-24 12:37:20 PDT
apparently this works (able to reproduce the failure) with the down arrow instead of the right arrow too.
Comment 14 :Ehsan Akhgari (busy, don't ask for review please) 2011-05-24 12:55:47 PDT
OK, this is totally a regression from bug 240933, and also happens in a normal textarea element.
Comment 15 Rob Campbell [:rc] (:robcee) 2011-05-24 13:01:57 PDT
we definitely want a unittest for this.

I can probably write one for the scratchpad pretty easily.
Comment 16 Rob Campbell [:rc] (:robcee) 2011-05-24 13:02:35 PDT
then again, it's also trivial to add one for data:text/html,<textarea/> too.
Comment 17 :Ehsan Akhgari (busy, don't ask for review please) 2011-05-24 14:14:54 PDT
Created attachment 534881 [details] [diff] [review]
Patch (v1)
Comment 18 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-24 15:33:44 PDT
Comment on attachment 534881 [details] [diff] [review]
Patch (v1)

Review of attachment 534881 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 19 :Ehsan Akhgari (busy, don't ask for review please) 2011-05-25 13:33:38 PDT
http://hg.mozilla.org/mozilla-central/rev/c4973e32a099
Comment 20 :Ehsan Akhgari (busy, don't ask for review please) 2011-05-25 13:34:19 PDT
(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) 2011-05-26 10:31:15 PDT
(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"

Note You need to log in before you can comment on or make changes to this bug.