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 1.0.1 Trigger Problem #1553

Closed
hmelman opened this issue Aug 4, 2013 · 26 comments
Closed

QS 1.0.1 Trigger Problem #1553

hmelman opened this issue Aug 4, 2013 · 26 comments

Comments

@hmelman
Copy link

@hmelman hmelman commented Aug 4, 2013

Since upgrading to 1.0.1 I've had some problems with triggers.

  1. Whenever I restart QS my Current Application Hide mouse trigger set to activate when mouse enters a corner is corrupt. I need to re-save it (selecting Current Application in the first pane) for it to work. It works until I quit QS and then always fails on restart.
  2. Each time I restart QS I've had problems with some web search triggers. These are in pairs, Search For... blank and Current Selection Find With... . The search is in the catalog in a custom web search catalog source. I have 7 pairs of these and I find on restart one or two pairs are corrupt and need to be re-saved. Sometime its the wikipedia pair that's broken sometimes it the amazon or google pair. When broken, the name of the trigger is corrupt, showing the full url instead of the name of search. This is fixed when re-saved.
  3. Often when activating a web search trigger such as Google Search For... the third pane is in search mode, not text mode. I type and it shows the big ? icon in the 3rd pane until I hit . to enter text mode. It works sometimes (starting the trigger with the 3rd pane active in text-mode) but if I esc esc out, it then fails over and over. I'm using the built-in bezel interface and have "Switch to text mode if no match is found" unchecked (though checking it doesn't seem to make a difference).

I'm going to guess it's something with proxy objects or maybe mouse triggers. I've saved my Triggers.plist when it's working and when it's failing and have posted a private gist I'll send to devs. I've deleted QS's caches and that didn't help. I'm curious if anyone can recreate the above triggers and have it work.

QS 1.0.1 (4001) 10.8.4, 3.4 GHz i7, 8GB RAM

@sinewave
Copy link

@sinewave sinewave commented Aug 4, 2013

I get #3 all the time. For me it happens whenever the last action i performed was a google search using a trigger and i am trying to perform the same action again.

If i do something else then it seems to "reset" and go back to normal.

-- 
Tony
On August 4, 2013 at 10:48:01 AM, hmelman (notifications@github.com) wrote:

Since upgrading to 1.0.1 I've had some problems with triggers.

Whenever I restart QS my Current Application Hide mouse trigger set to activate when mouse enters a corner is corrupt. I need to re-save it (selecting Current Application in the first pane) for it to work. It works until I quit QS and then always fails on restart.

Each time I restart QS I've had problems with some web search triggers. These are in pairs, Search For... blank and Current Selection Find With... . The search is in the catalog in a custom web search catalog source. I have 7 pairs of these and I find on restart one or two pairs are corrupt and need to be re-saved. Sometime its the wikipedia pair that's broken sometimes it the amazon or google pair. When broken, the name of the trigger is corrupt, showing the full url instead of the name of search. This is fixed when re-saved.

Often when activating a web search trigger such as Google Search For... the third pane is in search mode, not text mode. I type and it shows the big ? icon in the 3rd pane until I hit . to enter text mode. It works sometimes (starting the trigger with the 3rd pane active in text-mode) but if I esc esc out, it then fails over and over. I'm using the built-in bezel interface and have "Switch to text mode if no match is found" unchecked (though checking it doesn't seem to make a difference).

I'm going to guess it's something with proxy objects or maybe mouse triggers. I've saved my Triggers.plist when it's working and when it's failing and have posted a private gist I'll send to devs. I've deleted QS's caches and that didn't help. I'm curious if anyone can recreate the above triggers and have it work.

QS 1.0.1 (4001) 10.8.4, 3.4 GHz i7, 8GB RAM


Reply to this email directly or view it on GitHub.

@skurfer
Copy link
Member

@skurfer skurfer commented Aug 5, 2013

OK, at least one recent change (from #1505) is definitely causing problems here. The identifier for Current Application is changing from QSCurrentApplicationProxy to NSStringPboardType-:-QSCurrentApplicationProxy.

Just guessing, but maybe the triggers are being looked at before the Current Application proxy is added to the catalog. Since there's no object with that ID, it assumes it's just a string and adds the prefix. I actually have a trigger that's affected by this bug as well, but it's a rarely used event trigger.

This might also explain the web search problems, but that seems less likely, since the string sniffer should recognize them as URLs. In the example Gist, I only saw one web search that was different, and it had only been relocated in the list, not changed. (Since dictionaries are unordered, it's not unusual to see things move around like this.)

I can confirm that the 3rd pane doesn't automatically switch to text-entry mode if using a web search trigger was the last thing you did. The switch happens when QS thinks more information is required, and since you just populated the third pane, it probably thinks that requirement is already satisfied. But then, I would expect it to just run the trigger without showing the interface at all in that case, so I could be off.

@skurfer
Copy link
Member

@skurfer skurfer commented Aug 5, 2013

For @pjrobertson, I disabled the Event Triggers plug-in to force its proxies to be unrecognized and here's where the problems happens:

string_id_bug

I'm not sure if there's any good way to assign identifiers without causing a problem like this, since the assignment is happening so far away from the code that understands it's looking at a list of triggers.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Aug 5, 2013

Ok, good spot. Seems like we should make a. '-[QSObject _identifier]'
method for directly accessing the identifier without setting it. This
should be used in the '-[QSObject isEqual:]' method (and probably quite a
few other places). I'll sort it when I'm back.

P.S. iPod keyboards have no back tick?! Crazy

On Monday, August 5, 2013, Rob McBroom wrote:

For @pjrobertson https://github.com/pjrobertson, I disabled the Event
Triggers plug-in to force its proxies to be unrecognized and here's where
the problems happens:

[image: string_id_bug]https://f.cloud.github.com/assets/154676/910251/80f4aaae-fdd6-11e2-800b-c41eab0bdd20.png

I'm not sure if there's any good way to assign identifiers without causing
a problem like this, since the assignment is happening so far away from the
code that understands it's looking at a list of triggers.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1553#issuecomment-22107445
.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Aug 5, 2013

Oh - and thanks sine wave for giving a way to reproduce #3, I've seen this
a lot with existing builds, but have never known how to reproduce.

On Monday, August 5, 2013, Rob McBroom wrote:

For @pjrobertson https://github.com/pjrobertson, I disabled the Event
Triggers plug-in to force its proxies to be unrecognized and here's where
the problems happens:

[image: string_id_bug]https://f.cloud.github.com/assets/154676/910251/80f4aaae-fdd6-11e2-800b-c41eab0bdd20.png

I'm not sure if there's any good way to assign identifiers without causing
a problem like this, since the assignment is happening so far away from the
code that understands it's looking at a list of triggers.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1553#issuecomment-22107445
.

@skurfer
Copy link
Member

@skurfer skurfer commented Aug 6, 2013

P.S. iPod keyboards have no back tick?! Crazy

It's there, but not obvious. Hold down the ’ key for a second.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Aug 7, 2013

@skurfer
I can't seem to reproduce this, but take a look at the 'isequal' branch on the QS repo (not mine) and let me know if it helps. I'm not sure of the consequences of not resolving the identifier in isEqual - but potentially they're worse than the consequences of resolving the identifier (the consequences being this bug) - although I don't think so having done some testing

On 6 Awst 2013, at 20:42, Rob McBroom notifications@github.com wrote:

P.S. iPod keyboards have no back tick?! Crazy

It's there, but not obvious. Hold down the ’ key for a second.


Reply to this email directly or view it on GitHub.

@hmelman
Copy link
Author

@hmelman hmelman commented Aug 7, 2013

Thanks for looking into this (and no need to cc me directly, I follow qsdev :)

If there's more info I can provide to help, let me know.

Since QS doesn't crash often (well done on that) the Current App proxy is only a problem on restart. The text-entry mode thing I hit many times a day.

The web search problems do seem to affect just one web search trigger each time, though it varies. So on one restart it might be the google trigger that's broken, on another it's wikipedia, on another it's just amazon. And again in each case it's usual a single pair of triggers affected, the Google Search For... and the Current Selection Proxy Find With... Google triggers. Maybe it has something to do with the Current Selection Proxy which then breaks the web search url in the trigger which is a shared reference among triggers? (now I'm just making shit up :)

Howard

On Aug 5, 2013, at 9:36 AM, Rob McBroom notifications@github.com wrote:

OK, at least one recent change (from #1505) is definitely causing problems here. The identifier for Current Application is changing from QSCurrentApplicationProxy to NSStringPboardType-:-QSCurrentApplicationProxy.

Just guessing, but maybe the triggers are being looked at before the Current Application proxy is added to the catalog. Since there's no object with that ID, it assumes it's just a string and adds the prefix. I actually have a trigger that's affected by this bug as well, but it's a rarely used event trigger.

This might also explain the web search problems, but that seems less likely, since the string sniffer should recognize them as URLs. In the example Gist, I only saw one web search that was different, and it had only been relocated in the list, not changed. (Since dictionaries are unordered, it's not unusual to see things move around like this.)

I can confirm that the 3rd pane doesn't automatically switch to text-entry mode if using a web search trigger was the last thing you did. The switch happens when QS thinks more information is required, and since you just populated the third pane, it probably thinks that requirement is already satisfied. But then, I would expect it to just run the trigger without showing the interface at all in that case, so I could be off.


Reply to this email directly or view it on GitHub.

@skurfer
Copy link
Member

@skurfer skurfer commented Aug 12, 2013

This should probably be addressed before the next pre-release. Have you started on something @pjrobertson? If not, I can work on it in the morning.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Aug 12, 2013

Take a look at 5ef645f
The branch is called 'isequal' on the QS repo. I thought I pinged you somewhere, but maybe it got lost in the ether.

I didn't create a pull request as I said I couldn't reproduce the problem, so wanted you to test first. I've been running with it though, and it's fine.

…on hang on, I did mention this a few comments above: #1553 (comment)

On 12 Awst 2013, at 09:51, Rob McBroom notifications@github.com wrote:

This should probably be addressed before the next pre-release. Have you started on something @pjrobertson? If not, I can work on it in the morning.


Reply to this email directly or view it on GitHub.

@skurfer
Copy link
Member

@skurfer skurfer commented Aug 12, 2013

Yeah, you mentioned it. Sorry. So, that small change prevents trigger corruption if you don't go into the prefs and look at your triggers. But simply looking at them will result in several calls to identifier from various places. I started to change them, but in the end, I wonder if it would require less surgery to just make identifier return the value, and make a new method that tries to assign/alter it when necessary. Then we'd just need to find the places where it should be assigned/altered. I'm guessing that's a lot rarer than places that just want to retrieve it.

@skurfer
Copy link
Member

@skurfer skurfer commented Aug 12, 2013

BTW, an easy way to reproduce this is to create a trigger that uses a proxy object from one of the external plug-ins, then disable that plug-in. All of a sudden, the ID for that proxy looks like a string.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Aug 12, 2013

create a trigger that uses a proxy object from one of the external plug-ins, then disable that plug-in

Alright, I'll take a look later today. As for making a new method to set the identifier… that already exists (setIdentifier:) :P
It's debatable as to which method would be easier/less surgery. If we can make it so we only have identifier as the getter and setIdentifier: as the setter it would be cleaner, but I'm worried so many places blindly call identifier knowing they will get an identifier that it will brake (e.g. look at all our proxy objects. They call [someProxy identifier] == AnIdentifier. All these might break if the identifier isn't already set (I don't know if it will be)

On 12 Awst 2013, at 10:59, Rob McBroom notifications@github.com wrote:

BTW, an easy way to reproduce this is to create a trigger that uses a proxy object from one of the external plug-ins, then disable that plug-in. All of a sudden, the ID for that proxy looks like a string.


Reply to this email directly or view it on GitHub.

@hmelman
Copy link
Author

@hmelman hmelman commented Aug 17, 2013

Installed QS 1.1.0 (4003).
#1 seems fixed.
#2 I had problems with one web search (not a pair). It's now easy to spot, in the catalog it shows the full url in the trigger name and not the item name (e.g., "imdb").
#3 Still happens

@skurfer
Copy link
Member

@skurfer skurfer commented Aug 19, 2013

Just to make sure all of your corrupted triggers are OK now, can you open Triggers.plist and search for “NSStringPboardType-:-”? If you find it anywhere, just remove it and leave whatever comes after.

@petonic
Copy link

@petonic petonic commented Aug 30, 2013

Anyone have a copy of the 3942 build left? I have symptom #3 listed above, and it's really screwing up my workload. I'd rather go back to the last known copy of QS that works for this.

@skurfer: I did what you asked, and found one occurrence of “NSStringPboardType-:-”. I deleted that and left everything else (I deleted the whole node that contained that), but I still exhibit the same symptoms. I'm not sure who you were addressing when you posted that, so out of desperation, I tried it.

@skurfer
Copy link
Member

@skurfer skurfer commented Aug 31, 2013

We’ve figured out how to fix it, but it hasn't gone into a pre-release build yet. The last official release (4000 a.k.a. 1.0.0) shouldn't exhibit this problem, if you want to go back to that.

skurfer added a commit that referenced this issue Sep 3, 2013
* fixes a bug introduced by 4919bcd
* partially addresses #1553
@skurfer
Copy link
Member

@skurfer skurfer commented Sep 6, 2013

A new pre-release is out that should fix issue 3. Are you still seeing 2?

@skurfer skurfer closed this as completed in 36e6a03 Sep 6, 2013
@hmelman
Copy link
Author

@hmelman hmelman commented Sep 6, 2013

I installed 4004 and restarted and still saw #2. I fixed them, quit and restarted and found I still had problem #2.

#3 seems to be fixed so THANK YOU!

@skurfer
Copy link
Member

@skurfer skurfer commented Sep 6, 2013

At this point, is it affecting triggers where the search is in the first pane, third pane, or both? I wonder if it's a problem with "string sniffing". I noticed none of our unit tests for that test search URLs. Can you give me one that regularly fails so I can use that in coming up with tests?

@skurfer
Copy link
Member

@skurfer skurfer commented Sep 6, 2013

I've tried a few things and just can't get it to break. I made a Search For/Find With pair and restarted several times. I tried using the trigger with and without looking at the Trigger prefs first. I even tried manually replacing the search from my catalog with just the string for the search URL. Not only did it continue to work, but after a restart, it goes back to showing the thing from my catalog (instead of the URL), which is what I'd expect it to do.

Anything else you can tell me? Maybe you could do the before/after dance for Triggers.plist again, since one of the issues has been fixed since the last time. You know how to get it to me privately.

@petonic
Copy link

@petonic petonic commented Sep 6, 2013

Double confirmation that item 3 is fixed with the new build. Thank you, Skurfer, for the quick fix. I didn't even need to go backlevel. All is well with the world, at least for my use of Quicksilver.

@hmelman
Copy link
Author

@hmelman hmelman commented Sep 7, 2013

Reliably fails? Nope, as I said it varies each restart. It could be that the failures come from searches that are cataloged via a custom web search catalog source. Could that be the issue?

@hmelman
Copy link
Author

@hmelman hmelman commented Sep 7, 2013

Here's a screen shot with some bad ones, note the ones with URLS, ⌃⇧⌘W, ⌃⇧⌘S, ⌃⇧⌘E, ⌃⌘W

screen shot 2013-09-06 at 9 30 24 pm

@skurfer
Copy link
Member

@skurfer skurfer commented Sep 8, 2013

It's definitely not related to custom web search entries, because those are the only ones I use.

The only reason you'd see a URL there is if you manually added it in text-entry mode (which you didn't) or there's nothing in the catalog with that identifier. So, when you see a URL like that, could you go to the corresponding catalog entry and check the Contents tab to see if it's there?

I do see a possible bug with the Wikipedia URL. The ‘:’ prevents the string sniffer from recognizing it as a search URL. That would explain why it doesn't work if the thing isn't in the catalog. But when it does get added to the catalog correctly, the trigger should be fine.

I can't explain the others, though. They're recognized as search URLs by the string sniffer. I can disable my web search entry altogether and restart, but search triggers still work.

@skurfer
Copy link
Member

@skurfer skurfer commented Sep 9, 2013

I've submitted a fix for the bug with the Wikipedia URL, but the more I think about it, the more I think that shouldn't break the trigger. The bug would prevent the string from being recognized as a web search, but since the trigger explicitly specifies an action, it shouldn't depend on correctly determining type. So, it's an improvement, but unlikely to fix your specific problem. 😕

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

Successfully merging a pull request may close this issue.

5 participants