Tuukka Tolvanen (sp3000) Assignee
	Description • 23 years ago

BuildID: 2001112002 fwiw...

Loading urls by way of middle click in content area
("middlemouse.contentLoadURL") can sometimes get a bit irritating, and the
eventual implementing of autoscroll will propably make it a bunch less popular
as the middle-click action on content... Still, having some spot to paste an url
or keyword-bookmark into, to load directly, would be very nice under X, since
pasting into the urlbar itself is something of a chore under X compared to, say,
under Windows.

The bookmark/site icon next to the url text might be a good spot for this.

	Blu3
	Comment 1 • 23 years ago

I'd like that.  I'd also like consistency in the UI.

a) allow DND/paste of URL onto content whitespace from any location.  right now
you can only do it from a separate nav window
b) allow DND/paste onto TAB instead of just DND
c) allow DND/paste onto status bar instead of just DND
d) allow DND/paste onto location bar instead of just paste

How about highlight plaintext url and dragging that onto aforementioned items?

Any others?

	Akkana Peck
	Comment 2 • 23 years ago

How do we deal with the discoverability problem, if we change the middle-mouse
feature to only work in one tiny area?  If there's some way to learn about the
new behavior, I wouldn't have a problem with it, but I'm not sure how to address
that issue.

The icon next to the url is probably a bad place to put this, though.  First, we
should use an area that already exhibits the behavior, which that icon doesn't.
 (It probably should, but someone familiar with its XUL would have to implement
that.)  Aside from that, it's a bad substitute for another reason: it's a tiny
target and hard to hit, and missing it by a pixel on the right side pastes into
the urlbar, which is a bad failure mode since it screws up what's already there.
 (Admittedly, if you keep trying and eventually hit the target you'll end up
replacing the contents of the urlbar anyway, so maybe that's not so bad.  But it
being such a small target is bad.)

	Peter Trudelle
	Comment 3 • 23 years ago

->marlon for ue input

	Ben Bucksch (:BenB)
	Comment 4 • 23 years ago

*** Bug 137335 has been marked as a duplicate of this bug. ***

	Ben Bucksch (:BenB)
	Comment 5 • 23 years ago

A Right-click on the proxy icon should bring up a context menu, which contains
at least the menu items "Paste (load clipboard as URL)" and "File bookmark...".
That would solve the discoverability. IMO, the "discoverability" of the
paste-into-content is more harmful than useful - users trigger it accidently,
don't know what happened and think, it were are bug.

Compare esp. <http://bugzilla.mozilla.org/show_bug.cgi?id=85677#c24>.
This would solve or at least work around the problems that bug 24651 and bug
85677 are about.

	Tuukka Tolvanen (sp3000) Assignee
	Comment 6 • 23 years ago

The size matter: I was a little unsure of the icon being sufficiently large for
this originally, but testing with the current behaviour (er, since when did
middle-click and right-click bring up dialogs?) it seems good enough, and
certainly more convenient than manually cleaning up the urlbar via keyboard
before pasting (which kinda defeats the convenience of paste-with-mouse).

middlemouse.contentLoadURL is evil: It is *way* too error-prone, we actually
have plans to actively second-guess the user's intentions (perhaps they pasted a
password, or a poem? bug 85677). Because of this (and the *sigh* eventual
autoscroll), many will want to turn it thing off (bug 57317 implements the pref,
but unfortunately there is no ui for it). I think that middle mouse paste
loading an url directly is useful and convenient, but it does not belong in the
content of all web pages.

	Ben Bucksch (:BenB)
	Comment 7 • 23 years ago

Actually, I think that the 2 features, while similar, are really independant.
You can enable (by default or personally) both, one of them or none, and all of
that makes sense, at least to some people. So, for this bug, let's just assume
that that content-paste feature doesn't exist at all.

	Akkana Peck
	Comment 8 • 23 years ago

I don't get it.  Netscape 2-3-4.x on unix always had the content-middleclick
feature, but for some reason people are whining about it in Mozilla.  Why are
people so bothered about it in mozilla when they weren't in netscape?

	Tuukka Tolvanen (sp3000) Assignee
	Comment 9 • 23 years ago

Propably because middle-mouse wasn't used for anything else besides
middlemouse.contentLoadURL in Netscape 2-4 Unix (I don't know but I suspect that
is the case), now it's too overloaded with too different stuff (bug 70498).
Perhaps the slight difference in behaviour (bug 57317 comment 33) makes a small
difference too, but I suspect the former is the deciding factor. Anyway, this
bug is *not* about disabling middlemouse.contentLoadURL or providing pref ui for
doing so, but about providing a useful bit of ui functionality that can coexist
with middlemouse.contentLoadURL, and serve as a fallback for those who disable
middlemouse.contentLoadURL for whatever reason.

	Ben Bucksch (:BenB)
	Comment 10 • 23 years ago

> I don't get it.

See bug 85677.

> Netscape 2-3-4.x on unix always had the content-middleclick
> feature, but for some reason people are whining about it in Mozilla.

IIRC, we had this discussion already. At least Debian and Redhat must disable
that by default in their packages or there's anything else preventing it to
work, because it never worked for me on these systems, and I didn't change any
prefs to that matter.
Please let's focus this bug on the proxy icon only and not discuss here, if
content-paste is enabled or is a good idea, because a lot of people obviously
think that it isn't a good idea and disable it.

	Akkana Peck
	Comment 11 • 23 years ago

Ben: thanks for the pointer to that bug.  I wasn't even cc'ed on it, and I would
have been the logical person to fix it.  I'll look into it.

I didn't mean to imply that I wanted to block or hijack this bug.  This one
sounds like an excellent idea, and as you say, it doesn't depend on
middlemouse.contentLoadURL (it would be useful on all platforms regardless of
the default value of that pref).

	Tuukka Tolvanen (sp3000) Assignee
	Comment 12 • 23 years ago

it wfm, ship it

	Tuukka Tolvanen (sp3000) Assignee
	Comment 13 • 23 years ago

This patch is a minimalist approach, no context menu, just the rfe, no fries. ->me

	Ben Bucksch (:BenB)
	Comment 14 • 23 years ago

wfm, minimal impact, ship it (seriously, I do)

r=BenB
I can check in, if you have sufficient review (incl. superreview and approval of
a browser owner)

Wishlist:
- Add the context menu I mentioned above.
- Why a bookmark window should open on left-click on the proxy icon escapes me,
  but I guess that's the wrong bug.

	Tuukka Tolvanen (sp3000) Assignee
	Comment 15 • 22 years ago

the accidental newline deletion at the end of navigator.js no longer applied

	Tuukka Tolvanen (sp3000) Assignee
	Comment 16 • 22 years ago

Patch no longer applied (presumably due to fix of bug 158364).
Changes in this version:
  * includes fix to rfe bug 52784, except for the css change -- unavoidable
    since they go through the same click event, and bug 52784 is apparently
    going to be fixed
  * onclick is now on the <deck> instead of on both the <image>s in the
    <deck>. It appears to work fine in all cases. I propably put them on the
    <image>s originally because the drag handlers were there, or something
    like that.
  * randomly changed the click-handling function's name to something
    personally more pleasing on a whim, not that there's any common convention
    in the affected files
  * navigator.xul was missing "indent-tabs-mode: nil"

	Tuukka Tolvanen (sp3000) Assignee
	Comment 17 • 22 years ago

woohoo. That lasted for a neat 6 hours. Due to bug 105895 checkin,
middleMousePaste now wants an event, so it can do stuff depending on the
modifier.

	Vaclav Dvorak
	Comment 18 • 22 years ago

Any particular reason why the patch never attempted to get r/sr? Would be great
to have.

	Vaclav Dvorak
	Comment 19 • 22 years ago

This is an updated patch v2.1 that should apply cleanly to current tree. There
are no other changes to the previous patch than just shifting of line numbers.
It's made with Patch Maker and it's the very first patch that I'm submitting
for Mozilla, so forgive me if it's completely unusable. ;-)

NOTE:
The bug 52784 part works fine, but the part that is actually relevant for this
bug doesn't work for me in yesterday's nightly build on Win98. When I comment
out the try/catch in function readFromClipboard(), it gives the following JS
error:
Error: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIClipboard.getData]"  nsresult: "0x80004005
(NS_ERROR_FAILURE)"  location: "JS frame ::
chrome://navigator/content/navigator.js :: readFromClipboard :: line 1374" 
data: no]

That seems like a completely different bug, though.

	Vaclav Dvorak
	Comment 20 • 22 years ago

Ah, the error message that I'm getting is bug 58700. I'm reopening it and
marking it as blocking this.

	Vaclav Dvorak
	Comment 21 • 22 years ago

Comment on attachment 116038 [details] [diff] [review]
updated patch v2.1

I've tested the patch on Linux and it works like a charm. The clipboard JS
error that I mentioned above seems to be Windows-specific, and is tracked in
another bug.

Requesting r/sr from jag. (I've no experience with this, so sorry if it's not
the right procedure/person.)

	jag (Peter Annema)
	Comment 22 • 22 years ago

Do you really need the |case 0| part of the patch? The event should just bubble
up to the click handler on the containing autocomplete-textbox. For |case 1| you
should probably add event.preventBubble() so we don't select the urlbar text
(even though the selection will be cleared once the new URL is shown). Then you
might also want to add |onmousedown="event.preventBubble();"| to <deck> so you
don't confuse the focus code (well, not sure if that would, I'd have to study
the code a little more).

cc'ing Dean who wrote most of the urlbar click handling code.

	Vaclav Dvorak
	Comment 23 • 22 years ago

The |case 0| does an explicit select-all on the textfield. When we leave it out,
the textfield only selects all onClick when it doesn't yet have focus, and only
if the pref clickSelectsAll is true.

|onmousedown="event.preventBubble();"| - wouldn't that interfere with DnD?

	Dean Tessman
	Comment 24 • 22 years ago

So you're actually implementing two behaviors, one for left-button click and the
other for middle-button click?  Looks good to me, but I'll go ahead and cc:
neil, who re-wrote a lot of the urlbar click handling code.

	neil@parkwaycc.co.uk
	Comment 25 • 22 years ago

Nothing here looks like it will conflict with existing urlbar clicking.

	Vaclav Dvorak
	Comment 26 • 22 years ago

Thanks.

Dean: yes, see comment 16. The patch actually fixes this bug and bug 52784 (or
at least the more important part of it). I should note that my contribution is
infinitesimal, the work was done by Tuukka Tolvanen.

So, does it mean that the patch has review? How about super-review - should I
find someone else to do it or is the patch simple enough to get rs?

	jag (Peter Annema)
	Comment 27 • 22 years ago

Comment on attachment 116038 [details] [diff] [review]
updated patch v2.1

sr=jag

Neil, could you bless the patch with your r= then?

	neil@parkwaycc.co.uk
	Comment 28 • 22 years ago

I can do better than that :-)

	R.K.Aa.
	Comment 29 • 22 years ago

*** Bug 196940 has been marked as a duplicate of this bug. ***

	sairuh (rarely reading bugmail)
	Comment 30 • 22 years ago

vrfy'd fixed with 2003.04.02.08, linux rh8.0.