128953 - selection in textarea/input won't accept drops; doesn't remove selection

Status: NEW

(Core :: Layout: Form Controls, defect)

Description

[Asa Dotzler [:asa]]

If I delete a multiline entry in a text area (like the Additional comments box)
by highlighting it all and hitting the Backspace key I cannot drag and drop into
the text area. 

Steps to reproduce:

1. type a word in this comment box above, hit enter and type another word
2. click and drag to select the two lines
3. hit backspace
4. drag any link from the page into the textarea. 

Results: The drop does not put the link in the text area.
Additional info: if you type something in the text area it will again accept the
drop.

Possibly related or the same problem: if you have selected text in the text area
and attempt to drag and drop a link or other text into the area it does not
clear the current text area selection and replace it with the dropped text as is
expected. Instead it does nothing, leaving the old text highlighted and not
dropping anything new into the area. 

I see this consistently in Mac and Windows. I do not see this consistently in
Linux but I believe I was able to trigger it in linux at least once. 

Tested with 2002030409 0.9.9 build.

Comment 1

[/\/\arcio Galli]

I was able to reproduce the second case with linux: 

Which is:

(1) type in textarea or input text.
(2) select some text int he field.
(3) drag content from outside to the field. 
(!) does not work with:
    Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.8+) Gecko/20020304

Comment 2

[rods (gone)]

->>

Comment 3

[Kathleen :Brade]

I don't see the original bug as described in part 1.
I do see the 2nd part of the bug (if there is a selection present).

-->brade

Comment 4

[Martijn Wargers (dead)]

Attachment: patchv1

This patch just removes the selection when the text control gets blurred instead of making the selection invisible (which I find an odd thing anyway).
However, it always removes the selection, but it should not do that when you focus/select something in chrome or in a different document. In that case you should get a disabled selection. Not sure yet of how to do that. Maybe with the relatedTarget of the blur event, I guess.

I've also just removed a lot of code here:
-  the #ifdef USE_HACK_REPAINT code, that code seems just bogus. There are more places with those ifdefs. Maybe they can all be removed?
- if((flags & nsIPlaintextEditor::eEditorWidgetMask)  ||, etc
I don't really know what that code is doing, so maybe it is needed. Does anybody what it is needed for?

Comment 5

[Martijn Wargers (dead)]

Attachment: patchv2

Ok, this part: if((flags & nsIPlaintextEditor::eEditorWidgetMask)
is necessary for designMode iframes, which shouldn't get the selection cleared on blur.
I've put it back in in this patch.

Comment 6

[Martijn Wargers (dead)]

(In reply to comment #4)
> However, it always removes the selection, but it should not do that when you
> focus/select something in chrome or in a different document. In that case you
> should get a disabled selection. Not sure yet of how to do that. Maybe with the
> relatedTarget of the blur event, I guess.
Well, I simply don't know how to do that (not even sure if it is possible at all)
But I think patchv2 is at least an improvement over current situation.

Comment 7

[Kathleen :Brade]

Comment on attachment 207142 [details] [diff] [review]
patchv2

Do not remove the #ifdef USE_HACK_REPAINT.  It is unrelated to this bug.

I don't understand how this patch fixes the problem.  It seems like a workaround with a bad (IMO) side effect.  (I rely on seeing "disabled" selections.)

I would expect the fix to be in http://lxr.mozilla.org/seamonkey/source/editor/libeditor/text/nsPlaintextDataTransfer.cpp (and possibly also nsHTMLDataTransfer.cpp)

Comment 8

[Kathleen :Brade]

Comment on attachment 207142 [details] [diff] [review]
patchv2

Do not remove the #ifdef USE_HACK_REPAINT.  It is unrelated to this bug.

I don't understand how this patch fixes the problem.  It seems like a workaround with a bad (IMO) side effect.  (I rely on seeing "disabled" selections.)

I would expect the fix to be in http://lxr.mozilla.org/seamonkey/source/editor/libeditor/text/nsPlaintextDataTransfer.cpp (and possibly also nsHTMLDataTransfer.cpp)

Comment 9

[Kathleen :Brade]

A few more notes:

The bug probably lies in this area of the code:
http://lxr.mozilla.org/seamonkey/source/editor/libeditor/text/nsPlaintextDataTransfer.cpp#195

Around line 224 there is a check if the the mouse is over an existing selection (nsrange->IsPointInRange).  I'd guess that is where the log is flawed.  Try stepping through the code in a debugger (which I know is painful) or add some logging.

I notice with FF1.5 that I can drop text into a text area into text that is not selected (the drop caret won't appear until I move beyond the selection).  Please make sure that the caret feedback while dragging is also corrected and works with any future patches.

What is the expected behavior for dropping text over selected text?  Delete selection and then insert dropped text?  Reset the selection without deleting the selected text?

SubEthaEdit lets you drop the text wherever you place it (removing the old selection without deleting the text).  I don't happen to have MSWord, Dreamweaver or Notepad or others handy right now.

Thanks for working on this bug!

Comment 10

[Martijn Wargers (dead)]

The problem is this: When text controls are blurred, their selection becomes invisible. Then when you drag over that text control, you can't drop the text on the selection in the text control.
This is intended behavior, at least if I understand the comments in the source code correctly (see bug 101542, comment 17).
But now you have the situation that you can't drop anything on the text control because the text is selected, but the user can't see that the text is selected!

Imho, I think it is very strange to make the selection invisible, like Mozilla does. I think it is better, to just remove the selection, like IE6 does (only IE6 doesn't do that when you're switching focus into chrome, or to another window, I don't know how to do that in Mozilla).

I'm not sure what you mean with 'I rely on seeing "disabled" selections'
I'm not touching disabled selections (which you can see), I'm touching hidden selections (which you can't see).

Comment 11

[Martijn Wargers (dead)]

Brade, what do you mean with 'I rely on seeing "disabled" selections'?
With the patch, I'm not touching disabled selections (which you can see), I'm touching hidden selections (which you can't see).

Comment 12

[Kathleen :Brade]

>I rely on seeing "disabled" selections.

When I'm editing text, I like to see the selection in non-active edit fields.  For example, in this form, the "Additional Comments" is active and the Reassign field is inactive.  If I had a selection in the "Reassign" field, it would be nice to see it's outline (or the selection in light gray or similar depending on OS behaviors).  Long ago, I think Firefox supported this but I don't think it does anymore.

Comment 13

[Martijn Wargers (dead)]

I think I remember a bug (can't find the bug though) where that light gray for disabled selections in text fields was considered confusing.

In IE7, the selection seems to be removed in text fields (and also regular text). Except when the focus goes into chrome. In Safari on windows, that also seems to be the case. In fact. Except that regular text gets the light gray look when the focus goes to chrome.

Comment 14

[(no longer active)]

(In reply to comment #9)
> A few more notes:
> 
> The bug probably lies in this area of the code:
> http://lxr.mozilla.org/seamonkey/source/editor/libeditor/text/
> nsPlaintextDataTransfer.cpp#195
> 
> Around line 224 there is a check if the the mouse is over an existing
> selection (nsrange->IsPointInRange).  I'd guess that is where the log is
> flawed.  Try stepping through the code in a debugger (which I know is
> painful) or add some logging.

I think the right approach to fix this bug is to follow this comment.

Regarding invisible selections, I think we should keep the selection visible, as WebKit does, but that's probably the subject of another bug.

Comment 15

[Martijn Wargers (dead)]

Depending on which selection color you would choose for disabled selections in text boxes, this would be confusing or ugly. Also, it's not what IE is doing, except for content to chrome focus.
I would at least keep the caret visible while retaining selection then.

Comment 16

[(no longer active)]

(In reply to comment #15)
> Depending on which selection color you would choose for disabled selections in
> text boxes, this would be confusing or ugly. Also, it's not what IE is doing,
> except for content to chrome focus.

What version of IE did you try?  (I'm on Mac, so I wouldn't know!)

> I would at least keep the caret visible while retaining selection then.

We don't show the caret when there is a non-collapsed selection by default, so I'm not sure why we should start doing that for this case.

Comment 17

[Martijn Wargers (dead)]

I'm using IE9.
IE9 still has a blinking caret in chrome while some text is selected. When focus is removed, from the text box, the selection seems to be collapsed.(In reply to comment #16)

> (In reply to comment #15)
> We don't show the caret when there is a non-collapsed selection by default,
> so I'm not sure why we should start doing that for this case.

Because if there is no color difference for selection that has focus and selection that has no focus (disabled selection), then a user has no way of knowing what textbox has the focus.

But I guess you think that we should keep the disabled selection color (brownish in my theme)?

Comment 18

[(no longer active)]

(In reply to Martijn Wargers [:mw22] (QA - IRC nick: mw22) from comment #17)
> But I guess you think that we should keep the disabled selection color
> (brownish in my theme)?

Yes, that's what I'm suggesting.

Comment 19

[Phoenix]

Both original and Comment 1 scenarios are WFM, is this still actual?
User agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0 SeaMonkey/2.17a1
Build identifier: 20121202003011

Comment 20

[Phoenix]

Whoops, forgot to clean Crash Signature field after tests, sorry

Comment 21

[Alice0775 White]

I can still reproduce.
http://hg.mozilla.org/mozilla-central/rev/0352a32fde64
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121202 Firefox/20.0 ID:20121202030723

Steps to reproduce:
1. Type some text into a textarea
2. Select whole text in the textarea
3. Select some text in contents aera
4. Drag the selected text
5. try drop the dragging text into textarea

Actual results
Impossible to drop

Expected results:
I can drop and the text in the textarea should be replaced by the text that I dropped.

Comment 22

[Phoenix]

Ah, I see, it isn't allowed to drop on selection, but I can drop on some free place and selected text would be removed