Closed Bug 97207 Opened 23 years ago Closed 23 years ago

textarea pastes sometimes misplaced by failing to reposition caret

Categories

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

Product:
Core
Component:
DOM: Editor
Platform:
x86
Linux
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.9

People

(Reporter: jg, Assigned: kinmoz)

References

Details

(Whiteboard: EDITORBASE+) 

This is a spin-off from bug 57913.

The problem is that text is frustratingly pasted in a postion other than
expected, generally in the middle of some paragraph above the intended point.

What I observed:

1. Was writing paragraphs in a textarea
2. Pasted some text into the textarea at the end of my text, thus following it
   on.
3. Continued typing my own text for another paragraph
4. Pasted further text, again at the text's endpoint
5. Pasted text did not get inserted at the endpoint. Instead it was pasted
   above it, apparently at the beginning of the prior paste.
6. Performed Undo, ctrl+z. Paste was undone, and a cursor could be seen at the
   beginning of the last (correct) paste, confirming the cursor's role.

It appears that when we paste, we don't set the cursor at the position where the
mouse pointer is hovering. I should point out that I'm pasting by clicking the
middle mouse button.

Further experimentation by the community is encouraged to either confirm the
above, or get more accurate reports in of what exactly was the situation before
and after such a paste. CC dbaron as the owner of the other bug.
Well, AFAICT, the behaviour _is_ wierd. Why would ctrl-middle button insert to
the mouse position, however? The correct thing would be to paste where the
cursor is currently blinking, IMHO, which isn't what this bug specifies. What is
the correct behaviour? Well, at least in an xterm, insertion is done at the
cursor position. I find it more precise, and more expected.

Adding mpt for input focus opinion.
I've seen the behavior described here.  Several times, after seeing it, I have
attempted to recreate exactly what I was doing when it occurred, but the bug did
not recur.  I'll keep trying.

My gut instinct is to agree with Christian's comment, but:

1) Note that Xterm is not a good example: It is a terminal emulator.  The 
   application running within it does not know anything about mice, so 
   Xterm behaves as if the text was typed in at the keyboard -- and thus the 
   text appears at the cursor.

2) Xwindows applications are not consistent as to where pasted text appears.  
   However, most of the more modern apps insert pasted text at the location of 
   the mouse pointer, not the cursor.

   App                       Pastes at:
   ------------------------------------
   xedit (an XAW app)        cursor
   xephem (a Motif app)      cursor
   gnp (a gtk+ app)          pointer
   gedit (a gnome app)       pointer
   emacs (in X mode)         pointer
   Netscape 4.7              pointer
3) Text should appear where the user's attention is focused.  If the user is 
   clutching a rodent, he/she is most likely watching the mouse pointer, 
   not the cursor.
Retitling slightly.  The expectation that the middle click reposition the cursor
is a standard UI expectation of form widgets on X11 (in at least all the
toolkits I've used - GTK, QT, and Motif).  This bug is saying that that doesn't
happen only in certain rare situations, whereas it should happen 100% of the
time (rather than 95% of the time).

The one time I saw this happen since the fix for bug 57913 I think the paste
actually ended up somewhere other than the cursor was before I started the
paste, so I'm not even sure this description is right, or if this description is
the only remaining problem.  However, if the repositioning fails for some reason
(in the function that was changed in bug 57913), what would happen is that the
paste would occur at the old cursor position.
Summary: textarea pastes do not reposition cursor at pointer hover position (misplaces paste) → textarea pastes sometimes misplaced by failing to reposition cursor
-->akkana
If possible, you might try using multiple-undo and multiple-redo to see what is 
happening.
Assignee: brade → akkana
I suspect (and believe me, this is wild guess) that when I am typing following a
text paste, the position of the cursor becomes somewhat confused, and upon
subsequent pastes (the 2nd, at least), the cursor somehow returns to the start
of the first paste, then inserts the pasting text.

I may be way off the mark there. If it happens again I'll take particular
attention to cursor position.

Christian: Look again, and middle-clicking into the textarea DOES reposition the
cursor to the place under the pointer (well, when it works :).

*** Bug 96166 has been marked as a duplicate of this bug. ***
Can someone elaborate on how this is different from bug 57913?  It sounds like
the same issue to me -- layout not giving the right answer for
nsIDOMNSUIEvent::GetRangeParent.  Kin?  Do you know if this one is a different
issue?
bug 57913 was about how the code in nsFrame was returning the wrong content in
the blank area of a textarea, both after mjudge's commenting out of two key
lines, and before it, while lines overlapped if you had true type fonts (see bug
91794).  I suspect this is an actual editor problem.
middle-click paste = left click + paste isn't it?

David, I've seen your patch. A naive comment is why the middle button paste
function recalculates all the x-y positionning whilst the calculation could be
done calling lclick function ?

The two last functions behave correctly, afaik.
It should, yes, and I'm not sure why it doesn't work the same way, or where the
code that handles left clicks actually is.
Anthony says the event code needs to be changed to use the same positioning code
as left-click uses to place the caret, and he has a fix.
Assignee: akkana → anthonyd
--> kin
Assignee: anthonyd → kin
<lobbying>
Anthony, could be great to save time and fix 4xp bug 97053 at the same time,
since you modified the event handler (4 lclicks would call Select All) :)
</lobbying>
Tony--do you have a fix for this bug?  If so, could you attach it here or put it 
in the comments?  Thanks!
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9.6
Summary: textarea pastes sometimes misplaced by failing to reposition cursor → textarea pastes sometimes misplaced by failing to reposition caret
Are people still seeing this? I tried playing around with middle-mouse-paste in
my linux and Win32 builds, and each time I tried, it pasted wherever I clicked.

Is there a specific test case I should use? (ie. textarea in a table)
Whiteboard: EDITORBASE
Pushing this back to mozilla0.9.7 unless someone can give me a reproduceable
test case.
Target Milestone: mozilla0.9.6 → mozilla0.9.7
I'm seeing this in the URLbar in a yesterday afternoon build.

I know exactly what I did, but suspect it still won't be reproducible; I'll
record it here anyway before I quit and start trying to reproduce.

I ran the app, browser window came up, I called up my bookmark for "bug by
number" (brings up a dialog where I enter the bug number), middlemouse-pasted a
bug number into the text field, and clicked the button (usually I hit return but
I think I used the mouse this time).  The bug loaded.  I read it (scrolling down
with mousewheel and maybe also spacebar), then wanted to view another bug, so I
clicked in the middle of the url, typed ^E<BS><BS><BS><BS><BS> (usually I'd use
^H but this time I think I used BS), selected a bug number from an rxvt in
another window, positioned the mouse in the blank space after the end of the
text in the urlbar, and clicked the middle button.

The bug number pasted in the middle of "cgi", i.e. show_bug.cg12345i?id=

I clicked after the last pasted digit, hit five BSes and tried again
middle-clicking after the end of the line.  Same thing happened.

I speculated that it was pasting to the caret position rather than the pointer
(forgetting that that wouldn't have explained the first occurrence), typed ^E,
and tried again.  Still pasted before the question mark.

Now I've loaded this bug, typed all this stuff, and I still see it: click in the
urlbar, ^E followed by five backspaces, select a 5-digit number in another
(rxvt) window and middle click after the id=; the number pastes between "cg" and
"i".

Quitting now to restart and see if it continues to be reproducible.
Yes, fully reproducible.

- Run mozilla.
- Paste http://bugzilla.mozilla.org/show_bug.cgi?id=97207 into the content area
to go to the url.
- Left-click in the urlbar.
- Type ^E followed by five backspaces.
- Go to another window and select a 5-digit number.
- Middle click after the end of the url.

Sometimes it goes between cgi and ?, sometimes between cg and i, but never where
it's supposed to go.

Possibly relevant note: I have a userChrome.css which specifies the urlbar font
with the following lines:

input {
  font-family: clean !important;
}

I'm not currently resetting the size, just the family.  It's possible that
layout is making assumptions about the size of the font being used (IIRC, last I
looked, the default font was much smaller than this one as well as uglier).  If
you can't reproduce this I'll be happy to try again without the userChrome.css
and see if it still happens.
Some more info after playing around:

userChrome.css is not important -- I can repro without that file.

What is important is that you must start out with the window small enough that
the bugzilla url doesn't fit in the urlbar.  Size it so that you can't even see
the ?, so you have to make it horizontal-scroll to the end when you type ^E or
End.  The horizontal scrolling is confusing its idea of mouse event positions.

This happens in other text fields, too, not just the urlbar (e.g. try in the
"depends on" field of this bug, since that' one's fairly small).
It looks like this isn't an editor or selection problem ... the basic problem is 
that the point contained in the mouse event was not properly transformed so that 
it is in the same view coordinate system as the frame handling the event.

nsPresShell::HandleEvent() calls GetFrameForPoint(), with the point contained in 
the event structure, so that it can set mCurrentEventFrame. (mCurrentEventFrame 
is the frame that the presShell will use to dispatch/handle the event)

GetFrameForPoint() and GetFrameForPointUsing() don't seem to take into account 
crossing view boundaries. They both assume that as they pass the point down to 
child frames, that all they have to do to transform the point into the child 
frame's coordinate system, is to subtract off the the origin of the child frame. 
What I think really needs to be done is to adjust the point by also factoring in 
the position of the parent frame, like GetContentAndOffsetsFromPoint() does, 
before calling GetFrameForPoint() on the child.

After GetFrameForPoint() is fixed, we need to make sure that 
nsPresShell::HandleEvent() transforms the event point into mCurrentEventFrame's 
view coordinate system, before letting mCurrentEventFrame handle the event.

In akkana's test, it is just luck that we are getting the frame, from 
GetFrameForPoint(), we want to actually handle the event, but because the point 
is not in the correct coordinate system, we get the wrong offset into it.

GetFrameForPoint probably shouldn't descend into children that are in different
views (and I know in many cases where we know for sure that frames won't have
views, we don't, but we should really be more flexible about whether frames have
views).  nsPresShell::HandleEvent should be called once for each view that might
contain the point, until there actually is a frame that contains it.  (We
currently get the order slightly wrong -- see bug 96832 and bug 49175.)
Kin: could I put in a request that this be elevated above P3?  I don't see it
often (though I guess some people do), but when it happens, it makes it
completely impossible to paste.  Just now I was trying to update a bug and after
about 15 tries I finally gave up and typed in the text I was going to paste
(fortunately it was only a moderately complex url, not a novel :-)
--> 0.9.8
Target Milestone: mozilla0.9.7 → mozilla0.9.8
Suggest WONTFIX. Paste should not move the cursor at all. This isn't standard
UNIX behavior. I've filed bug 119075 in a fit of rage after Mozilla pasted
paragraphs of text all over random sentances that were already there. More
reasons are there.

Christian Reis all the way back in addition comment #1 had it right, "The
correct thing would be to paste where the cursor is currently blinking". If I
want to move the cursor, that's what left mouse button is for. Not moving the
cursor gives you a garuntee of exactly where the text will go. You can left
click or cursor exactly where you want it, rather then try to target between to
letters, or on a new line, and having to make all kinds of changes manually if
you miss.
Just how often have you used unix?  Did you ever use emacs?  Or netscape 4.7x? 
They all paste at the point you click the middle button, not not where the
cursor was before.
> Just how often have you used unix?

Every day (minus a few vacations) for the last 6 years.

> emacs?

EMACS, vi, and friends all paste where the cursor is, as I run them in terminal
mode. So does my mail client, and news, and IRC, and every other app I use.

> netscape 4.7x?

Yes. It's textfield paste behavior was the second most annoying thing in
Netscape Classic.

It only took a few hours after this bug was filed for someone to comment that it
shouldn't move the cursor, so I'm obviously not the only one who feels this way.
If you have to make middle-click do the equivilent of a left-click, at least
give us a hidden pref. I can't stand having to perfectly target my middle-clicks
with no way to verify where it's going before the text insert.
All you've told us is that terminal apps (xterm and friends) don't paste to the
mouse cursor position.  However, EVERY gui app on unix does paste to the mouse
cursor position in editable fields, and has for over a decade.

In any case, it's obviously bogus that it pastes to random positions, which is
the problem in this bug (it doesn't paste to the caret position, but to some
random place; moving the caret to the desired paste location before doing the
paste does not make this bug go away).
> All you've told us is that terminal apps don't paste to the mouse cursor

Well, other a browser, that's all I use. As do alot of other UNIX users.
(windows kiddies running Linux to be cool don't count). I've heard it said that
 X was originally written with only xterm, xclock, and xload in mind. Probably
not true, but it sure would explain a lot.

> However, EVERY gui app on unix does paste to the mouse cursor position in
> editable fields, and has for over a decade.

"EVERY" is a lot of apps, and "decade" is a long time. Let's see... "xedit".
nope, pastes to the text cursor. "xclipboard". nope, pastes to the text cursor.
"xterm", well, we covered that. "xman". nope, pastes to the text cursor. I think
you'll find every "official" (ships with) X app pastes to the cursor.

Oh, do you mean GNOME? The desktop built on top of a toolkit designed for a
graphics program? WELL THEN! I'm sure it's text handling is impeccable! You
don't seriously suggest GNOME/gtk developers have a clue about UI?

A pref should be trivial to add around the code that moves the cursor before
text insert. I think it would make a lot of UNIX users happy. (I'm done).
Please take the X vs GTK discussions to another bug.
This bug isn't about where paste should go to where the caret is or where the
mouse is.  That belongs in a *new* bug.  

Jeremy--Please file a new bug.

This bug *IS* about pasted text not going to where the caret or the mouse is (a
RANDOM! location!!!)  This bug is particularly nasty because it isn't
predictable (no matter which behavior you prefer).
Target Milestone: mozilla0.9.8 → mozilla0.9.9
plussing
Keywords: nsbeta1
Whiteboard: EDITORBASE → EDITORBASE+
Depends on: 83650
I just attatched a patch to bug 83650 that hopefully fixes this problem. Anyone
want to try it out? Bug 83650 also contains some test case that can help you
easily reproduce the problem where clicking places the caret somewhere else.
Keywords: nsbeta1+
*** Bug 123926 has been marked as a duplicate of this bug. ***
Fix for 83650 checked into TRUNK, it should also fix this bug:

  mozilla/layout/base/public/nsIFrame.h              revision: 3.194
  mozilla/layout/html/base/src/nsContainerFrame.cpp  revision: 1.132
  mozilla/layout/html/base/src/nsFrame.cpp           revision: 3.359
  mozilla/layout/html/base/src/nsFrame.h             revision: 3.164
  mozilla/layout/html/base/src/nsPresShell.cpp       revision: 3.509
  mozilla/layout/xul/base/src/nsBoxFrame.cpp         revision: 1.180

Should appear in 02/08/02 QA builds.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
I am pretty sure this fix caused, or revealed, bug 124485.

I don't get this comment:
>>> GetFrameForPoint() and GetFrameForPointUsing() don't seem to take into
account crossing view boundaries. They both assume that as they pass the point
down to child frames, that all they have to do to transform the point into the
child frame's coordinate system, is to subtract off the the origin of the child
frame. <<<

That should be quite OK. If not, then there is a bug in layout and views aren't
being positioned correctly.

>>> What I think really needs to be done is to adjust the point by also
factoring in the position of the parent frame, like
GetContentAndOffsetsFromPoint() does, before calling GetFrameForPoint() on the
child. <<<

I don't understand why this should be necessary. The view manager always passes
in a point in the coordinates of the frame's view, which is relative to the
frame's origin. Your assertion here
http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsPresShell.cpp#5901
verifies that the view and frame match.

Can you describe the configuration of views and frames that was causing a
problem with the old code and that your patch corrects for?
> I am pretty sure this fix caused, or revealed, bug 124485.

Yeah it did, apparently a frame can contain a view that is not a descendant of
the frame's parent view. I've posted a fix for this, which basically returns an
offset of (0,0) in nsFrame::GetOriginToViewOffset() when this case is detected.

> That should be quite OK. If not, then there is a bug in layout and views aren't
> being positioned correctly.


No the positions of the views are ok. Child frames are positioned relative to
their parent frames, *unless*, their parent frame contains a view (that is
parentFrame->GetView() actually returns a non-null pointer), in which case the
child frames are relative to that view.

This is how we get scrolling to happen ... the view contained by the frame has
it's position changed which results in all of the child frames shifting, giving
the scrolling affect.


> I don't understand why this should be necessary. The view manager always
> passes in a point in the coordinates of the frame's view, which is relative
> to the frame's origin. Your assertion here
> http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsPresShell.cpp#5901
> verifies that the view and frame match.


Right they do match but apparently the assumption that view was a descendant of
the frame's parentView was wrong, when dealing with menus.


> Can you describe the configuration of views and frames that was causing a
> problem with the old code and that your patch corrects for?


I was trying to correct the assumption that you can't map a point in a given
frame's coordinate system, to it's child coordinate system, by simply
subtracting off the upper left corner of the child frame, because the upper left
corner of the child frame could be relative to something other than it's parent
frame, like a view.


> This is how we get scrolling to happen ... the view contained by the frame
> has it's position changed which results in all of the child frames shifting,
> giving the scrolling affect.

Oh, I thought we had a child frame holding the scrolling stuff whose origin
moved as we scroll, but I believe you when you say that we don't. :-)

> Right they do match but apparently the assumption that view was a descendant
> of the frame's parentView was wrong, when dealing with menus.

Yeah. I just realized that it's also not true for other popups and even for
fixed-position elements. My bad. I seem to be having a very bad day.
James, this should be working for you now...please verify...let us know...

thanks.
*** Bug 127889 has been marked as a duplicate of this bug. ***
*** Bug 128073 has been marked as a duplicate of this bug. ***
*** Bug 128592 has been marked as a duplicate of this bug. ***
jg, does this work for you now?
verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.