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)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED WONTFIX
mozilla1.1beta

People

(Reporter: cesarb, Assigned: akkzilla)

References

()

Details

Attachments

(1 file, 3 obsolete files)

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: