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)
Tracking
(Not tracked)
VERIFIED
WONTFIX
mozilla1.1beta
People
(Reporter: cesarb, Assigned: akkzilla)
References
()
Details
Attachments
(1 file, 3 obsolete files)
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.
Comment 1•23 years ago
|
||
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
Comment 2•23 years ago
|
||
alecf/beppe, would this be an issue with selection? who should get this bug?
Severity: enhancement → normal
Comment 3•23 years ago
|
||
who worked on the paste to url feature?
Updated•23 years ago
|
Target Milestone: --- → mozilla1.2
Comment 5•23 years ago
|
||
Just ignoring pasted stuff with spaces would break very useful keyword
bookmarks, such as "bug 85677", which I middle-click-paste rather often.
Updated•23 years ago
|
Status: NEW → ASSIGNED
Target Milestone: mozilla1.2 → Future
Reporter | ||
Comment 6•23 years ago
|
||
I think it's possible to check for keyword bookmarks before checking for
spaces... And it would work as you expect then.
Comment 7•23 years ago
|
||
removing myself from the cc list
Comment 8•23 years ago
|
||
*** Bug 135884 has been marked as a duplicate of this bug. ***
Comment 9•23 years ago
|
||
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?
Comment 10•23 years ago
|
||
Ah, bzbarsky said
user_pref("middlemouse.contentLoadURL", false);
in bug 135884, thanks...
Comment 11•23 years ago
|
||
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.
Comment 12•23 years ago
|
||
Just ignoring stuff with newlines would be a good start. I have this pref turned
off for that reason.
Comment 13•23 years ago
|
||
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.
Comment 14•23 years ago
|
||
> 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.
Comment 15•23 years ago
|
||
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.
Comment 16•23 years ago
|
||
can we:
check if the url contains spaces, if it does:
check if it's a bookmark, if not:
don't load
else load
Reporter | ||
Comment 17•23 years ago
|
||
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.
Reporter | ||
Comment 18•23 years ago
|
||
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.
Comment 19•23 years ago
|
||
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)
Reporter | ||
Comment 20•23 years ago
|
||
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).
Comment 21•23 years ago
|
||
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?
Comment 22•23 years ago
|
||
> 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).
Reporter | ||
Comment 23•23 years ago
|
||
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)?
Comment 24•23 years ago
|
||
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.
Comment 25•23 years ago
|
||
> - 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....
Reporter | ||
Comment 26•23 years ago
|
||
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.
Comment 27•23 years ago
|
||
> 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.
Comment 28•23 years ago
|
||
Well, I find the button in bug 24651 unusable small as well, so... :)
Comment 29•23 years ago
|
||
*** Bug 128389 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 30•22 years ago
|
||
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
Assignee | ||
Comment 31•22 years ago
|
||
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.
Assignee | ||
Comment 32•22 years ago
|
||
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).
Reporter | ||
Comment 33•22 years ago
|
||
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...
Comment 34•22 years ago
|
||
> 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!
Comment 35•22 years ago
|
||
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()).
Reporter | ||
Comment 36•22 years ago
|
||
> 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.
Comment 37•22 years ago
|
||
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.
Comment 38•22 years ago
|
||
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)?
Assignee | ||
Comment 39•22 years ago
|
||
"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.
Comment 40•22 years ago
|
||
> 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.
Assignee | ||
Comment 41•22 years ago
|
||
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?
Comment 42•22 years ago
|
||
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.
Assignee | ||
Updated•22 years ago
|
Attachment #87624 -
Attachment is obsolete: true
Assignee | ||
Comment 43•22 years ago
|
||
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
Comment 44•22 years ago
|
||
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>
Comment 45•22 years ago
|
||
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.
Comment 46•22 years ago
|
||
> 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.
Comment 47•22 years ago
|
||
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.
Comment 48•22 years ago
|
||
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.
Comment 49•22 years ago
|
||
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?
Assignee | ||
Comment 50•22 years ago
|
||
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
Comment 51•22 years ago
|
||
> 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.
Comment 52•22 years ago
|
||
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.
Comment 53•22 years ago
|
||
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.
Comment 54•22 years ago
|
||
> 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.
Assignee | ||
Comment 55•22 years ago
|
||
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
Comment 56•22 years ago
|
||
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.
Comment 57•22 years ago
|
||
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.
Comment 58•22 years ago
|
||
mass-verifying Wontfix bugs.
mail filter string for bugspam: Tursiopstruncatus
Status: RESOLVED → VERIFIED
Updated•20 years ago
|
Product: Core → Mozilla Application Suite
You need to log in
before you can comment on or make changes to this bug.
Description
•