Closed Bug 23669 Opened 25 years ago Closed 24 years ago

Middle-mouse paste into text field can load a URL

Categories

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

Product:
Core
Component:
DOM: Editor
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: mcafee, Assigned: joki)

References

Details

Attachments

(2 files)

Patch to the ender event listener
2.03 KB, patch
Details | Diff | Splinter Review
workaround patch to check for input fields
980 bytes, patch
Details | Diff | Splinter Review 
linux
bugzilla, new bug page
Do not give the window focus anywhere
select "rgoodger@ihug.co.nz" from an xterm,
middle-mouse paste into To: field for bug,
page tries to load "rgoodger@ihug.co.nz"
heh, I had it load the page pasting the email address
to write this bug too, this paste was into the comment
text area.
Status: NEW → ASSIGNED
Target Milestone: M13
Kin's been seeing this, too, but I didn't have a way to reproduce it on linux so
I thought it was a windows event problem.

I'll get right on it.
OS: other → All
Hardware: Other → All
Saari and I have been trying to trace this down.

nsEnderEventListener::DispatchMouseEvent gets the event first, makes a copy of
the event, and calls a bunch of dom event routines on it (none of which do
anything immediately, and commenting them out don't stop the behavior).

Then nsTextEditorMouseListener::MouseDown (a listener on the editor document
inside the text field) gets the event (saari thinks it's getting the copy of the
event, not the original event).  If you trace this in the debugger, you can
trace it down to the Paste() call, and then as soon as you hit the Paste() call,
the JS routines get called even though we aren't finished handling the event on
the editor document yet and thus have had no chance to mark the event has
handled.

Clipboard paste is asynchronous on Unix, so the nsClipboard code called from
Paste() has to run a loop in order to wait for the contents of the clipboard.
But that doesn't explain what's happening, because even if you comment out the
call to Paste(), the same thing still happens -- the JS is called before the
editor event listener has had a chance to handle the event completely.

We tried inserting a
      if(event.flags & NS_EVENT_FLAG_STOP_DISPATCH) {
        aKeyEvent->PreventCapture();
        aKeyEvent->PreventBubble();
      }
in nsEnderEventListener::DispatchMouseEvent, like the nsEnderEventListener key
handling routines have, and preventing bubble/capture from the editor event
listener, but it doesn't help, because the mouse event on the window is passed
through to JS before the editor listener has a chance to call
PreventBubble/PreventCapture.

We need to find out why the event handlers on the window are being called so
early, before the event handlers on the editor document inside the widget have
had a chance to complete.  Saari is going to look at this on Windows.
Assignee: akkana → pavlov
Status: ASSIGNED → NEW
It gets more complicated.

First, there are two or possibly more bugs.

First, Saari pointed out that nsEnderEventListener::DispatchMouseEvent seems to
dispatch to JS events on the window before it dispatches to the editor event
handler.  That doesn't seem right, and I've tried switching the order of these
two lines (will attach a patch).  This fixes the problem on Windows, but not on
Linux.  Cc'ing buster because he owns the event handler and I'm hoping he'll
have a chance to look at the patch and say whether it's okay, or whether there
was a reason for doing things in the order they were done.  Since it doesn't
actually solve the problem on Linux, I'm not in a hurry to check it in right
away.

Second, since the ender event listener builds up a false event and passes that
through to the text editor event listener, it's not entirely clear that the
original event ever gets marked as having been processed, and we also never call
anything like PreventBubble() on it.  So it's puzzling why making the change
described in the previous paragraph would have fixed the problem on windows.

Third, and most difficult, the patch doesn't fix the problem on Linux,
apparently because of the asynchronous nature of Paste.  What happens is that
the event comes in and gets dispatched eventually to the editor event listener,
which calls the clipboard, which then calls gtk_main_iteration_do to wait for
the X server to return the clipboard data.  But meanwhile, the mouse up has come
in, and the gtk event handler sees it, so the ViewManager gets called to process
the mouse up, and somehow we end up in JS handling the previous mouse down event
which we haven't finished handling yet.  (And actually, it's even worse, because
the JS also asks for the clipboard data, so we call the clipboard and the gtk
event loop recursively.)

Pav says this is a known problem with Paste, and something he needs to fix
anyway, so I'm assigning to him.  Unhappily, I'm not sure that the problem lies
entirely in the clipboard's event loop, because it turns out that commenting out
Paste in the text editor event listener does not stop this problem from
happening.  Nor does commenting out all four of the steps for handling the event
in nsEnderEventListener::DispatchMouseEvent.  It's not at all why these events
are getting through at all, since I can comment out everything that saari thinks
should be responsible for dispatching the event and somehow both events still
get through.  There's something very wonky in our handling of events in gtk.
Oops, forgot to add myself to the cc.
Assignee: pavlov → akkana
Target Milestone: M13
reassigning to akkana since I have no idea what is going on.
oh wait.  i see.  i'm filing a new bug against me saying that i need to fix some
things in the way pasting is handled
Assignee: akkana → pavlov
Pav just agreed to look at this again.  (We'll probably both be investigating in
different ways.)
*** Bug 24026 has been marked as a duplicate of this bug. ***
Assignee: pavlov → joki
Target Milestone: M13
We really need joki to help look at this.  Even if we comment out the call to
Paste(), so we're not going through the X event loop code, the event still gets
handled twice on Unix.  I can help with debugging, but neither Pav nor I know
enough about how events are supposed to work to be very effective at figuring
out what's going wrong here.

This makes paste unusable in text fields and text areas for Unix users, so we
need to find either a solution or a workaround for M13.
*** Bug 24142 has been marked as a duplicate of this bug. ***
Status: NEW → ASSIGNED
Target Milestone: M13 → M14
Workaround has been checked in.  We now prevent navigation to a new url via a
middle mouse click if the click occurs over a text field.

Long term we need to work out the issues involved in getting events back and
forth between ender text widgets and the main document.
the text control synthesizes a new event and passes it down to the embedded
webshell.  IF we can get to lightweight webshells (no native widget), it's
possible this won't be necessary any longer.  Kevin is leading the charge there.

If we don't get to lightweight webshells, or if we don't get there soon enough,
then maybe all we need is to check some state from the synthesized event and
propogate that state back onto the original event?
On windows NT, should middle click even be paste? and if so, middle clicking on
the rest of the page still tries to load whatever you pasted as an URL. That's
quite annoying if you're using a mousewheel and you accidentally press a little
too hard. Suddenly I get an alert with two pages of text...
sorry about that URL, I had accidentally pasted in my clipboard into the URL field.
URL: You should only need to overwrite the...
Going out on a bit of a limb here... I'm wondering if it is possible that this 
is in part a diguised version of bug 9701, "nsWebShell Key events not working 
until focus set in window (TAB, PgDn, ALT+, + CTRL+)", NEW, M14. 

It has been mentioned above that it looks like the event is getting copied.
If the first copy is going to the browser page object, because the click that
is meant to do the paste is first giving focus to the page, this might explain
the URL load: IIRC, pasting a URL into the page should load that URL. 

In any case it may be worth trying to reproduce this on a build with the
workaround removed by copying, left-clicking in the edit field to give focus,
then middle-mouse-button-pasting. If the page load dose not happen then, the
fix for bug 9701 should make the workaround irrelevant.

Part of this might also be equivalent to bug 23336, "Double paste on single 
middle mouse click", NEW, if the above reasoning makes any sense.

*** Bug 25642 has been marked as a duplicate of this bug. ***
*** Bug 26113 has been marked as a duplicate of this bug. ***
As reported in the duplicate bug reports, the workaround checked in 1/18 does
not treat password fields.  Pasting into <input type="password"> fields still
loads the password as URL -- which isn't good since the password thus appears in
plain text.
I'm attaching a patch for navigator.js which also checks for type PASSWORD.  Can
someone review this patch?  If it works, I'll check it in.
*** Bug 25513 has been marked as a duplicate of this bug. ***
Just in case anyone hadn't realized, this bug broke off bug 15275, which 
introduced the middle-mouse-click-to-go-to-URL feature; there are implementation 
details in that bug report which may or may not be useful in fixing this bug. 
The original suggestion was for this feature only to work on Unix, where it was 
4.xP.

Besides form elements, the paste-and-go should be blocked from working in 
applets or plug-in areas (which may have their own uses for middle-clicks).
No longer blocks: 25129
The actual bug here is that the event should be caught by the lower-level widget
and should never bubble up to the level where the javascript acts on it.  So it
shouldn't be necessary to check for applets, etc., etc. as the workaround code
does ... the event system should take care of all of these cases, once this bug
is fixed.
Akkana: what about <input type=file> which uses a text control as well?  if you 
want to check this in as a workaround, fine, but don't mark this bug fixed.  I'd 
leave it open (or submit a new bug, your choice) so the real fix gets done 
properly.
*** Bug 26880 has been marked as a duplicate of this bug. ***
Blocks: 15275
Had a discussion with hyatt today about how we can solve this long term.  Have 
some ideas but none easy enough for M14.  Moving to M15.
Target Milestone: M14 → M15
1. The workaround prevents this from working not only over <INPUT type=text>,
but over plain old text. Is there anything one can do with them middle mouse
button that would interact with middle-mouse-click-load-URL?

2. Since you can now define your own handlers for the onclick event using
javascript, whatever you do should wait for the onclick handler for the element
the cursor is over, and not load the URL if the handler returns false  (see bug
28604: context menu comes up regardless of what the onclick handler returns).
Mass-moving bugs out of M15 that I won't get to.  Will refit individual 
milestones after moving them.
Target Milestone: M15 → M16
Blocks: 36922
M16 has been out for a while now, these bugs target milestones need to be 
updated.
This bug needs re-evaluation in the new world of Ender-lite text widgets.
Updating Milestone to M18.
Target Milestone: M16 → M18
I believe that the event model has been fixed.  Events now go to the right
place, and the hacky fix has been removed.  Marking fixed.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
(QA: please just verify that middle-mouse does the right thing -- pastes if over
a text control, scrolls if over a scrollbar, goes to url if over anything else
-- and take my word for it that the event model is better and the workaround is
gone.)
verified in 10/30 build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: