Closed Bug 85677 Opened 23 years ago Closed 22 years ago

middle-click in the content area thinks everything is a URL

Categories

(SeaMonkey :: UI Design, defect)

Product:
SeaMonkey
Component:
UI Design
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED WONTFIX
Milestone:
mozilla1.1beta

People

(Reporter: cesarb, Assigned: akkzilla)

References

(
URL
)

Details

Attachments

(1 file, 3 obsolete files)

Trim ws before word-splitting; allow up to 6 words
2.20 KB, patch
Details | Diff | Splinter Review 
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux 2.4.5 i686; en-US; rv:0.9.1+) Gecko/20010612
BuildID:    2001061208

When accidentally using middle-click in the content area, if you have random
text selected, it treats it as a URL, even if it makes no sense at all.

Reproducible: Always
Steps to Reproduce:
1.Make sure you have the paste-in-content-area-goes-to-URLbar misfeature enabled
2.Select something long and dumb, like "Mozilla 0.9.1 released" with the mouse
3.Middle click in the background

Actual Results:  "mozilla 0.9.1 released could not be found. Please check the
name and try again."

Error loading URL http://mozilla%200.9.1%20released/ : 2152398878
in the console

Expected Results:  Noticed it (a) is random selected text, and not a URL (b) it
isn't even legal URL syntax (come on, spaces???) and (c) it's pretty obvious the
user misclicked, since if he wanted to load a URL, he would have selected one first.

It should either just feep, or do nothing.

Fixing this would reduce a great amount of the annoyance level of this misfeature.

I know there's a hidden pref to avoid it, but (a) lusers won't know about it and
(b) people who actually use it would love if it didn't try to treat longish
texts in spanish as if they were URLs (or even something one might type in the
URL bar; SPACES is the key here!)

Just filter on spaces which are not in the beginning or the end of the text,
that'll cover 90% of the cases.
Over to XP apps.  Sounds like a good idea (and I _like_ the paste-URL feature).
Assignee: asa → pchen
Status: UNCONFIRMED → NEW
Component: Browser-General → XP Apps
Ever confirmed: true
QA Contact: doronr → sairuh
alecf/beppe, would this be an issue with selection? who should get this bug?
Severity: enhancement → normal
Blocks: 86180
who worked on the paste to url feature?
Blocks: 97651
-> blake
Assignee: pchen → blakeross
Target Milestone: --- → mozilla1.2
Just ignoring pasted stuff with spaces would break very useful keyword
bookmarks, such as "bug 85677", which I middle-click-paste rather often.
Status: NEW → ASSIGNED
Target Milestone: mozilla1.2 → Future
I think it's possible to check for keyword bookmarks before checking for
spaces... And it would work as you expect then.
removing myself from the cc list
*** Bug 135884 has been marked as a duplicate of this bug. ***
Someone said:

> I know there's a hidden pref to avoid it, but (a) lusers won't know about it

This luser would like to know about it -- is there
some way I can turn this paste behavior off altogether?

Ah, bzbarsky said
  user_pref("middlemouse.contentLoadURL", false);
in bug 135884, thanks...

This bug is security relevant. I occasionally paste passwords for website
accounts using middle-click into textboxes, and (lat least for some time)
happened rather often that Mozilla ended up loading the passowrd as URL. Duh! Of
course, that exposes the password to the network and the DNS server, without any
SSL protection.

My suggestion is: disable the paste into the content area altogether. Then make
the proxy icon in the urlbar active in the way the content area is now. This
keeps the convience, dramaticaqlly reduces the wrong triggers and doesn't
confuse users.
Just ignoring stuff with newlines would be a good start. I have this pref turned
off for that reason.
Contra to #11. 
Working with passwords via copy&paste is a user error.
This shouldn't be an argument to cut off a very usable feature. This might not be
that important for Win, but for X middle mouse click is pasting, and it's a good
thing that this works on more than just a tiny little area.
Especially for those of us, who hate to have windows raise on focus.
> that important for Win, but for X middle mouse click is pasting,
> and it's a good thing that this works on more than just a tiny
> little area.  Especially for those of us, who hate to have
> windows raise on focus.

Well, I use X, and I hate to have windows raise on focus, and 
I think this behavior is totally nonintuitive and awful -- if I
wanted to paste into the URL field, I would have pasted there.

I second jwz, and Ben in #11: this *is* security-relevant.  This bug has been
plaguing my life for some time now, although I never *imagined* it was a
*feature*.  And, yes, I usually get sensitive stuff sent to the DNS server this
way.  Not necessarily passwords, but certainly whatever I happen to be working
on at the time --- I don't believe that my current workspace should be being
leaked to the name server and the local server by this (highly effective) covert
channel.

I'm going to recommend debian include the magic user_pref in their default
mozilla packaging.
can we:
check if the url contains spaces, if it does:
check if it's a bookmark, if not:
don't load

else load
Won't solve the problem with passwords in comment 11, since it's rare for a
password to have spaces.

The algorithm should trim leading and trailing whitespace (including newlines!),
and use the resulting text as the URL. This works around copying more than you
want (it's common to copy the preceding or following spaces, copy a whole line
complete with \n, or copy with borken programs *ahem*mozilla*ahem* which insert
extra newlines after/before the copy).

Then, to check if it's valid:
1. Check for keyword bookmarks
2. Check if it has at least one dot before the first / (or if there's no /, in
the whole word; ^[^/]+\.[^/]+(/.*)?$ is the regexp)
3. Check if it begins with \w+: (a word followed by a colon; ^\w+:.+$ )

If it isn't any of the three cases, it's probably not a URL; feep and ignore the
paste request (I HATE popping dialog boxen).

If it pass any of these tests, then it's something that we can mangle into a URL
without looking absurd; go on.

This should work most of the time. Comments?

P.S.: Allowing ^/.*$ as a fourth test would also allow pasting of local
pathnames; I don't think it's a great idea, but... ^\w:[\\/].*$ is the one for
Win, I don't know the right one for Mac. Win also has ^\\\\[^\\]+\\.*$ which is
a network share.
Ah, forgot one thing: if it doesn't pass step 1 and has any whitespace, it's not
a URL and it shouldn't even bother doing the other tests.
Dots aren't needed for a valid URL.

And, re comment 11, it's impossible to distinguish a password from a hostname
(unless it contains special chars which might be forbidden in domain names/URLs)
Dots aren't required.

The schema part (the one like http: ) is.

www.mozilla.org isn't a valid URL.

http://www.mozilla.org is.

servidor isn't a valid URL.

http://servidor is.

When we find something without the schema, we have to guess. The standard guess
is to prepend http://, or ftp:// if it starts with "ftp." .

So, there's no problem requring the dots when doing a paste into the content
area, since this would reduce a lot the chance of mispasting a password (most
passwords don't have dots; it's not perfect, but it's better than nothing).
Yeah... pasting hostnames w/o dot and w/o http: in front of it is probably a
rare case.

So:
1) Check if it's a bookmark (if yes, load it)
2) Check if a space is found (if yes, abort)
3) Check if a dot is found (if no, abort)
4) Check if it's a local file (?) (if yes, load it)
5) Load the url

Did I summarize this correctly?
> pasting hostnames w/o dot and w/o http: in front of it is probably a
> rare case.

Except on intranets, where it happens all the time.

> 3) Check if a dot is found (if no, abort)

I'll buy checking that either a dot or a scheme is present....  Just checking
for a dot makes intranet urls very hard to deal with.  (and don't forget
about:cache, about:config, etc).

Re comment 21: You forgot 3.5) check for a scheme.

Re comment 22: I know that on a intranet single-word hostnames are common. But
if we want to prevent inadvertent middle-paste of passwords, we have to at least
forbid anything without a dot or a slash. So, would checking for ^\w+:.*$ OR
^[^/]+/.*$  be enough? (First is something:*, second is something/*). I think
that not working in the corner case of pasting a single word, as long as it's
documented as a feature, would fix about 90% of the password mispastes while
working with 100% of the URL pastes and about 90% of the "guessable" pseudo-URLs.

Let's add another test for ^[^@]+@[^@]+$ (email, bug 86180)?
Maybe my last comment got lost:
Please get rid of that load-after-paste-into-content-area *completely* and
replace it with load-after-paste-into-proxy-icon.
(The proxy icon is the one in the urlbar, which stands for the current page.
It's either the bookmark icon or the favicon.)
- A middle-click on that proxy icon is sufficient for power-users
  to conviently load a uri that is in the (primary) clipboard.
- It won't confuse new users like the other solutions do.
- It does not have unfortunate side effects like the password one I mentioned.
- It is easier to implement than solution proposed in comment 21.
> - A middle-click on that proxy icon is sufficient for power-users
>  to conviently load a uri that is in the (primary) clipboard.

It's also a tiny target, especially on a hi-resolution screen.  It would be
_very_ hard to use.

The regexps proposed in comment #23 make sense to me....
Re comment 24: - It's another bug

Believe me, I tried. I also saw that as a stupid thing to do (and I still seem
it as stupid, even after getting used to it and using it all the time). It was
shot down.

If you want to try, feel free to file another bug. It'll get duped against the
same WONTFIX bug.

Since I can't make them remove the misfeature, I can at least try to make it be
a bit less of a misfeature. Both sides win with it.
> It's also a tiny target, especially on a hi-resolution screen.

I have a 1600 screen and it would be usable for me. The button implemented in
bug 24651 is the same size and people seem to be able to use it.

> Re comment 24: - It's another bug

OK, filed bug 137335.
Well, I find the button in bug 24651 unusable small as well, so... :)
*** Bug 128389 has been marked as a duplicate of this bug. ***
Taking this.  I have a fix -- it's not perfect but it should take care of many
cases.
Assignee: blaker → akkana
Status: ASSIGNED → NEW
Target Milestone: Future → mozilla1.1beta
Attached patch Patch, first round (obsolete) — — Splinter Review 
Here's my initial take.  The very simple algorithm is: if it has more than one
word (space-delimited), and a colon isn't one of the first eight characters,
then it's probably not a url.

Suggestions for other things to check would be welcome.  Failing that, I could
use a review.
Attached patch Patch for this plus bug 105895 (obsolete) — — Splinter Review 
Here's a patch which combines this fix with the fix for bug 105895 (in case
anyone wants to either test or review them together).
1. You forgot possible leading and trailing whitespaces
2. You forgot the valid case "www.mozilla.org"
3. Same for "/tmp/empty.html"
4. Same for "\\server\share\file.txt"
5. You forgot keyword bookmarks
6. The list goes on and on and on...
> 1. You forgot possible leading and trailing whitespaces

Yeah, that would be a good thing to strip out...

Items 2--5 are correctly handled by that patch already.
Item 6 is kinda vague.  An actual example that fails with that patch?

Akkana, barring the leading/trailing whitespace issue that looks great!
I think this is pretty much a power user feature, and I haven't had any trouble
with it (but then again I ctrl-click links, instead of middle-clicking them). If
there's anything we need to change, it should be changing the default value to
turn this off, and put a comment about it in the release notes so the power
users will know how to turn it on (there's a bug on this somewhere).

That said, this patch doesn't seem to work with keyword bookmarks ("bug 85677")
or something like "jaggernaut:password@ftp.foopy.com". I suggest you alter the
logic slightly: first call |url = getShortCutOrURI(url);|, if that's not null,
construct a nsStandardURL and set |stdUrl.spec = url;|. If that doesn't fail,
then |loadURI(url);| (and do away with isProbablyURL()).
> Items 2--5 are correctly handled by that patch already.

comment #5 has an example of a keyword bookmark, "bug 85677". I think it won't
work (but I don't even claim to understand keyword bookmarks).

Yeah, I misread the patch the first time (&& not ||). Sorry. (And I can't see
why ^.{0,8}:.*\w.*$ being valid would be useful. Proper URLs don't have any
embedded spaces.)

Also, does the patch prevent "Hv6Dhgjm" from being treated as a valid URL? It's
a password, and shouldn't be valid.
Hv6Dhgjm is most definately a valid url (as long as http:// is assumed)

Will this patch leave non URLs -> keyword server intact? It's quite nice to be
able to highlight a few words (in any application), then middle click to search
google for them.
Jag, shouldn't the getShortCutOrURI be handling the bookmark keyword case?

Would |stdUrl.spec = "yahoo"| fail or succeed?

Cesar, how do you tell "Hv6Dhgjm" apart from "yahoo" (which should imo load
http://www.yahoo.com as it would if it were typed in the URL bar)?
"bug 12345" works fine: it expands keywords first, before applying the filter,
so any keywords will already be a proper url.  (I definitely wouldn't want to do
anything to prevent "bug 12345" from working -- I use that many times every
day.)  It would probably be more helpful if people criticising the patch would
either read it, or try it, first.

I like the idea of leading and trailing spaces invalidating the url.  I'll
attach a new patch that adds that.

jaggernaut:password@ftp.foopy.com is treated as a url.  That's correct, IMO, but
I'm not sure whether jag was agreeing or disagreeing with that.

Re embedded spaces: I know proper urls aren't supposed to have them, but in
practice they're pretty common, especially in urls sent via mail (which is a
common source for urls to middle click) and we really should handle them.

I don't know whether this will leave non-URLs -> keyword intact: I have that off
myself (because I didn't know until now about being able to customize
keyword.URL -- neat!)  I'll try it and get back to you.  I'll try to make it
work -- we wouldn't want to lose that.
> I like the idea of leading and trailing spaces invalidating the url.

I really really REALLY don't. I often select a url and occasionally grab a space
on one side of it accidently. Makes perfect sense that it's striped currently.

It would be incredibly frustrating to not be able to paste " http://foo.com",
have to go to a text editor, paste, reselect, paste back into moz.
Attached patch Combined patch, stripping spaces (obsolete) — — Splinter Review 
I'm happy to report that keyword searches still work with this change, so we
don't have to worry about that problem.

Here's a combined patch that strips spaces.  Either this invalidates 87607, or
vice versa (but I won't mark either one obsolete until we resolve the
space-stripping issue).

Jeremy does have a point.  I, too, frequently accidentally highlight spaces; I
was thinking that they already didn't work for middleclick in content, but it
turns out that's only true for bookmark shortcuts.  In other words, if I select
" bug 12345" and middleclick, it tries to load that as a url rather than as a
bookmark.  But if I select " http://www.mozilla.org" and paste it, it works
fine (prior to this patch, but not after it).

Also, before the patch, if I highlight "bug 12345 " and middleclick it, it
works fine, whereas after the patch it no longer works.  So even before I read
Jeremy's objection and tried a url, I was thinking about arguing for allowing
trailing spaces even if we removed leading spaces.

But now my vote is to allow urls with spaces on either end (and invalidate the
patch I am now attaching).  After listening to my arguments and Jeremy's, do
the rest still feel strongly that leading and/or trailing spaces should not be
allowed?
We should certainly allow leading/trailing space... The point I was trying to
make is that we should strip such space before splitting on " " to test for the
number of words....  Think someone pasting in "yahoo " -- we want that to work.
Attachment #87624 - Attachment is obsolete: true
Whoops, keywords weren't always working.  I've improved the word-splitting
smarts, trimmed leading/trailing whitespace before splitting into words, and
allow up to six words without a : (i.e. up to six words might be a keyword
search or a url with filenames with spaces).
Attachment #87604 - Attachment is obsolete: true
Attachment #87607 - Attachment is obsolete: true
This logic seems to be getting pretty complicated... can someone clue me in as
to why url bar entry and middle-click entry can't share a code path for all of
this? Seems like they should behave the same way for parsing everything.

<Greedy>This might also fix a RFE I filed years ago about having middle-click
URL entries being added to the url bar history pulldown, since middle-click is
just an (unbelivably cool, IE-killing) shortcut to ctrl+L, delete, mouse aim,
middle-click, enter.</Greedy>
bzbarsky: in the old patch, getShortcutOrURI was never reached because we would
return before that because of the check in isProbablyURL.

Akkana: I thought jaggernaut:password@.... would fail your isProbablyURL check
(colon > 8) but that's only when your word count is more than 6 (misread && for
||), so you're fine there.

JMD: it's because you can actually paste a paragraph of text into the URL bar
and press enter and it will try to load it as is. The only difference there is
that you will see your mistake before you hit enter, and correct it, an
opportunity you don't have with middle-clicking in the content area.

-----

I'm not convinced we need to do this. The logic is arbitrary (why 6? why 8?) in
determining what's valid middle-click pastable content and what's not. Can we
really make this logic be smart enough to account for all cases a user might
have come to expect to work? Currently there's no limitation, with the trade-off
that a user can middle-click paste something that they didn't intend to be
loaded as a URL.

If you're a user who knows about this feature then I assert you have no problem
at all realizing what just happened and recovering from it (you'll either have
to go back a page or click away the dialog). The problem I see is for people who
don't know about this feature and accidentily run into this when they misclick
(when wanting to open a link in a new window/tab), which is why I propose this
be disabled by default.

It's a cool feature, but it's not obvious and a problem for new users, because
even with this guard (or any reasonable guard, which doesn't include checking
for and only allowing http://, ftp:// etc.) in place there are going to be
plenty of text selections that will slip through the test and be loaded (or
attempted to) as urls. And if we do disable this by default, I don't think
there's a problem anymore.
> I propose [middle-click in contentArea] be disabled by default.

I'm willing to concede that may be preferable (no pun intended) to a lot of
users, with the clause that there should be a highly visable UI'd pref for
turning it back on, at least in UNIX. Many many users... maybe less then 50%,
but many, know, use, and love this feature. These users are generally savvy
enough to browse through the prefs to look for an option, but if all we do is
throw a note in the 1.1beta release notes:
  hey, there's a new hidden pref you need to enable now to get the behavior
  netscape 4/6/7 and mozilla have had for the last SEVEN years. close mozilla,
  open this file in an editor, add this, do this, for all your profiles...
we're going to have alot of confused, complaining people.
Netscape 4.x did not have this feature on Unix.

I don't doubt that Netscape 6.x did, and I'm sure the 5 people
who used that alpha release will be able to figure it out.
4.x *certainly* did. It was the only reason I prefered NS4 over IE4/5/6. I'm
using it right now. This is 4.7x, but, I know it was in 4.5, and I'm pretty sure
in 4.0 as well.
Wow, you're right, I just tried.

I can't understand how I *never* noticed this in 4.x, but got screwed by it on a
regular basis in Mozilla until I turned it off.

Perhaps it's that the text fields were easier to hit with the mouse in 4.x for
some reason, so I just never happened to miss?
I'm fairly sure it worked in NS 3 as well.  Not sure about 2.x or earlier.

I'm not sure why people seem to be tripping over it now when it's a feature
that's been around so long; perhaps earlier Netscapes guarded against triggering
it on strings that obviously weren't urls.  Hence this bug.
Status: NEW → ASSIGNED
> I'm fairly sure it worked in NS 3 as well.  Not sure about 2.x or earlier.

Definitely not in 3.x or 2.x (I just tried.)

> I'm not sure why people seem to be tripping over it now 

Me neither, but I'm leaning toward the text-field issue.

In both 3.x and 4.x, there was what looks like 5 or 6 pixels of inner margin
inside text field boxes, whereas in Mozilla there's what looks like about 2
pixels.  They were quite a bit taller before Mozilla, almost twice as tall
depending on the font size, so that would have made them a lot easier to click
on without missing.

4.79 appears to only trigger on text selected from a text field, not form the
rest of the page. I only have a two button mouse, and my 'middle click' is
emulated; not sure if that affects this behaviour.
As far as I know, 4.x doesn't do any "is this a url?" checking on the text. It
just tried to load "ainst the evils " and ended up passing it along to
search.netscape.com.

4.x tries to be a little clever though in not allowing links or text copied from
the current window to be middlemouse-pasted. I actually find this annoying, but
it may have helped.

> Many many users... maybe less then 50%, but many, know, use, and love this 
> feature.

Where can I find this information?

You're right that we probably should make this a visible pref somewhere if we
turn this off by default.
> 4.x tries to be a little clever though in not allowing links or text copied from
> the current window to be middlemouse-pasted. I actually find this annoying

Oh no. Please don't anyone get the idea to consider that as a solution here.
That was one of the most annoying 4.x bug/features for me. Had to paste into an
xterm, select again, and paste back.

> As far as I know, 4.x doesn't do any "is this a url?" checking on the text.

I pasted in a whole paragraph of text... it all got forwarded to search.netscape.
Well, it sounds like the consensus is that we shouldn't have code to try to
guess whether the clipboard contents are a url.

So I think that means this bug is a wontfix.  I'm sure people will speak up if
I'm misinterpreting, and set me straight as to exactly what this bug should now
cover, and whether anyone actually wants my patch or a derivative of it.

There's been some talk (here and in other bugs) of exposing the pref, and most
people seem in favor of that (though there's no obvious consensus as to the
default).  Is there a bug tracking that?
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → WONTFIX
akk, I would guess this bug is the bug for it. It is the catching bug for those
people annoyed about the current behaviour. The heurisitics were just *one* of
the proposed solutions, and not a good idea IMO. Other solutions would be a pref
(where the default should be to not load IMO), or getting rid of the code
altogether in favor of bug 111337 only.

FWIW, I use the paste-to-proxy-icon in bug 111337 (with middleclick-on-content
paste disabled) since some months now on a 19" hires screen, and it works just
fine for me. No problems to hit the icon - copying the exact URL is usually what
costs time. IMO, the problem would be just user education.

So, I suggest to reopen the bug to discuss/persue other options to solve the
problem.
Oh, rereading the initial description, you are right. Bug 135884 were just
wrongly marked dup of this one. I will reopen that bug, and we can track other
options there.
mass-verifying Wontfix bugs.

mail filter string for bugspam: Tursiopstruncatus
Status: RESOLVED → VERIFIED
Product: Core → Mozilla Application Suite
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: