Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QS assuming text is URLs #214

Closed
pjrobertson opened this issue Apr 20, 2011 · 13 comments
Closed

QS assuming text is URLs #214

pjrobertson opened this issue Apr 20, 2011 · 13 comments
Assignees
Labels

Comments

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Apr 20, 2011

First off, here are some problems:

  1. Enter text mode, and type 'define:hello'
    QS thinks this is a URL because of the semi-colon
  2. Enter text mode and type 'what.is.this!'
    QS thinks it's a URL again

Secondly, QS is returning text to the 1st pane, and thinking its a URL (even when the two above isn't true)
Where I cam across it:
Install the Web Search Module
Enter text in the 1st pane (e.g. quicksilver) -> Find With -> Any search engine you have
Reopen QS. The text you entered is in the 1st pane as a text object, but only has URL actions

This second problem is most likely a problem with the web search plugin itself. I know @skurfer has been dealing with returning things to the 1st pane recently, so he may have some ideas on this.

The 1st problem is most likely to do with what's written in QSObject_StringHandling.m:59
We need a more rigorous test to see if it's a url.

My suggestions for string tests are:
if it contains ://
if it contains 2 or more dots
if it contains www.
if it contains typical tlds?
e.g. .com,.co.uk,.org etc. etc. - this may be a tough one to validate with so many tlds in existence.

@HenningJ
Copy link
Contributor

@HenningJ HenningJ commented Apr 20, 2011

I don't think we can come up with something better to identify URLs than Apple did.

NSURL *url = [NSURL URLWithString:urlString];

checks if urlString is a vaild URL. Together with testing against [url scheme] and/or [url host] should be better than anything we can come up with.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 20, 2011

That's already in there. There's already a test for:

[url scheme](not [url host])

What I'm thinking about is e.g. you type:

google.com
or www.google.com
NSURL URLWithString returns nothing for those.

Alcor had implemented some kind of URL checking system to do with '.' and
'/' and stuff, but it lets too much stuff through

On 20 April 2011 20:55, HenningJ <
reply@reply.github.com>wrote:

I don't think we can come up with something better to identify URLs than
Apple did.

   NSURL *url = [NSURL URLWithString:urlString];

checks if urlString is a vaild URL. Together with testing against [url scheme] and/or [url host] should be better than anything we can come up
with.

Reply to this email directly or view it on GitHub:
#214 (comment)

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 20, 2011

My solution for the 1st part is to create an array with all TLDs from: http://data.iana.org/TLD/tlds-alpha-by-domain.txt

QS then compares the last component of the string (if it contains . and has at least 4 components) to see if it's one of these.
I've basically just added another check to Alcor's already implemented check.
Although the array has something like 270 TLDs, I've seen no slow down whilst typing text on my 4 yr old MacBook :)

I'll submit the code for discussion/merging (so you can see it) if nobody has any more suggestions/ideas

@HenningJ
Copy link
Contributor

@HenningJ HenningJ commented Apr 20, 2011

That's already in there. There's already a test for:

[url scheme](not [url host])

What I'm thinking about is e.g. you type:

google.com
or www.google.com
NSURL URLWithString returns nothing for those.

Alcor had implemented some kind of URL checking system to do with '.' and
'/' and stuff, but it lets too much stuff through

right you are. BUT...there is no way to really make sure that
something is a URL. It always too greedy (identifying strings as URLs
when they aren't meant to be) AS WELL AS not greedy enough. Believe
me, I've tried. :-)

But of course you are welcome to try. :-)

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 20, 2011

Alcor was 99% there, I've just done another test to see if the last thing
after a '.' is a TLD. (still validates if the last thing is a file extension

I haven't found a problem with it yet!

On 20 April 2011 21:20, HenningJ <
reply@reply.github.com>wrote:

That's already in there. There's already a test for:

[url scheme](not [url host])

What I'm thinking about is e.g. you type:

google.com
or www.google.com
NSURL URLWithString returns nothing for those.

Alcor had implemented some kind of URL checking system to do with '.' and
'/' and stuff, but it lets too much stuff through

right you are. BUT...there is no way to really make sure that
something is a URL. It always too greedy (identifying strings as URLs
when they aren't meant to be) AS WELL AS not greedy enough. Believe
me, I've tried. :-)

But of course you are welcome to try. :-)

Reply to this email directly or view it on GitHub:
#214 (comment)

@HenningJ
Copy link
Contributor

@HenningJ HenningJ commented Apr 20, 2011

If it works...great. :-)

@skurfer
Copy link
Member

@skurfer skurfer commented Apr 20, 2011

This has always bothered me. Glad to see it getting some attention.

In the interface, it’s more of an annoyance, while in the clipboard history it’s actually broken.

If you grab some text like “this.com”, it thinks it’s a URL and shows those actions by default, but the text actions should all still be available, shouldn’t they? What do you mean by “only has URL actions”? When I grab a selection that QS thinks is a URL, not only can I run “Large Text” on it, but I’ll only see the actual text as selected.

Here’s what’s broken: If I copy “this.com” to the clipboard, then use the clipboard history to paste it, I’ll end up with “http://this.com”, which is obviously wrong.

Something else I don’t quite understand from your initial report: Are you saying that after doing a web search, the search terms in the first pane are treated like a URL? Unconditionally?

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 20, 2011

Maybe it's just me, but if I copy 'this.com' to my QS clipboard, copy some
other items, then paste it/drag it I still get the same thing. No http://

RE the example of search terms. It's not the returned objects that are being
seen as URLs, but the initial search term itself.

e.g.
'quicksilver' -> find with -> Google
This opens the search URL in your browser and hides QS.
Open QS again, and 'quicksilver' is still in the 1st pref. But it's treated
as a URL. Strange

On 20 April 2011 23:02, skurfer <
reply@reply.github.com>wrote:

This has always bothered me. Glad to see it getting some attention.

In the interface, its more of an annoyance, while in the clipboard history
its actually broken.

If you grab some text like this.com, it thinks its a URL and shows
those actions by default, but the text actions should all still be
available, shouldnt they? What do you mean by only has URL actions? When
I grab a selection that QS thinks is a URL, not only can I run Large Text
on it, but Ill only see the actual text as selected.

Heres whats broken: If I copy this.com to the clipboard, then use the
clipboard history to paste it, Ill end up with http://this.com, which
is obviously wrong.

Something else I dont quite understand from your initial report: Are you
saying that after doing a web search, the search terms in the first pane
are treated like a URL? Unconditionally?

Reply to this email directly or view it on GitHub:
#214 (comment)

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 20, 2011

Oops - and I've just realised typing define:hello is still seen as a URL :(

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 20, 2011

OK fixed the define:hello bug. That was the main annoyance with this in the first place. Can't believe I forgot about this!

@skurfer
Copy link
Member

@skurfer skurfer commented Apr 20, 2011

Maybe it's just me, but if I copy 'this.com' to my QS clipboard, copy some
other items, then paste it/drag it I still get the same thing. No http://

Yeah, it was just you because you were running on code you just wrote. ;) I’m testing from your pull request and now the text gets pasted correctly. This has been a pain in my ass for years.

It still looks like a URL in the history, and the second line is prefixed with “http://”, but we can file all sorts of issues on the clipboard module.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 20, 2011

Yay fixed more than one bug in one go! Me likes :) I thought I was testing
it with an old QS, but I guess not!

One more commit to make then it should be able to be merged.

On 20 April 2011 23:58, skurfer <
reply@reply.github.com>wrote:

Maybe it's just me, but if I copy 'this.com' to my QS clipboard, copy
some
other items, then paste it/drag it I still get the same thing. No http://

Yeah, it was just you because you were running on code you just wrote. ;)
Im testing from your pull request and now the text gets pasted correctly.
This has been a pain in my ass for years.

It still looks like a URL in the history, and the second line is prefixed
with http://, but we can file all sorts of issues on the clipboard
module.

Reply to this email directly or view it on GitHub:
#214 (comment)

@ghost ghost assigned pjrobertson Apr 24, 2011
@HenningJ
Copy link
Contributor

@HenningJ HenningJ commented Apr 29, 2011

Fixed in pull request #238

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants