Creating trigger with only an action kills trigger editing UI

[deweeeese]

version: Quicksilver b58
host: MacOS X 10.6.3
machine: MacBook Pro 2007

repro:

1. install fresh copy of Quicksilver b58, never used before on this machine
2. from menu icon, select "Triggers..."
3. select Custom Triggers
4. add a hot key trigger, click on Action, enter anything (e.g. 'asdf'), save it
5. after saving, the trigger should disappear (a validity check seems to have deleted it)
6. now, add another new hotkey trigger
7. BUG: note that two new empty hotkeys are added immediately, and you can no longer edit triggers unless you quit and restart the application.

For a noob like myself, this was KILLER, I had no idea what was going on!
Thanks, -- jd

[pjrobertson]

Confirmed

[flyin1600]

I was able to repo this in b58 as well, but I have been trying to reproduce this issue in b59 and it does not seem to happen anymore.

When I click "save" on a new Hotkey trigger with only an Action, there is no new trigger in the trigger pref pane and if I click Add again i get the Command Builder window and I don't get 2 empty triggers in the qstriggersprefpane window.

One Issue i do see, is that if i click straight into the Action QSSearchView object, when i begin typing i get the same results as if you click in the "Select an Item" QSCollectingSearchView object. That doesn't seem to make sense for the Action box because those are actually items which you want to perform an action on, not actions themselves.

Is there any reason the Action box should not have behavior like the "Target" box? By which I mean that the "Target" box does not see to be modifiable unless you have a valid Item and Action selected. Maybe it would be better if you could not act on the "Action" box until you have an item selected in the "Select and Item" box.

What do you think?

[pjrobertson]

You're right, this shouldn't be the case.

We'll look at trying to fix this.

So does the original bug you reported work fine for you now?

[flyin1600]

yeah, with your and rob's help i was able to get my first build of QS up and running in the debugger. thanks again!

i've been trying to look a little deeper into the issue described here and i think is see what i happening. When the QSCommandBuilder window is opened the searchObjectChanged function in QSCommandBuilder is called (i think due to a notification being issued when setObjectValue is called when the QSCollectingSearchObjectView is created as part of the window). That calls the searchObjectChanged function in the super class which is QSInterfaceController.

In QSInterfaceController's searchObjectChanged function, it checks to see why it's getting the notification. It should be because of a change in one of the three parts of the comman: dSelector (direct target), aSelector (action), or indirect target (iSelector).

To do that it checks to see if the notification's object (instance of QSCollectingSearchObjectView) is equal either dSelector, aSelector, or iSelector. In this case the object contained by the notification is nil because the window is being created and nothing has been populated yet. Because the window is just being opened and hasn't been populated with anything, that actually makes the nil object equal to dSelector, so it enters the if case that is intended to match only if we have a valid direct target to act on. in that if check the aSelector text box is prematurely enabled.

that's the cause of the strange behavior inside this window. I'm pretty sure it's the cause of the original problem described by this issue even though it's not happening exactly the same with this new version.

i believe this could be fixed with a simple check in QSInterfaceController:searchObjectChanged (line 363) to verify that the object inside the notification is not nil
[[notif object] objectValue]

[pjrobertson]

I think you're on the right track with this.

What I've seen though is that searchObjectChanged only searches when the
object is changed. Maybe what should be done is that the TAB action (or
click action) does nothing until there's an object in the first pane.

Maybe by going into the .xib and seeing if there's a command for something
when the 2nd pane is actived

e.g. (made up terms)
secondPaneDidActive:

You could then do a check:
if(![aSelector objectValue])
go back to first pane
else
continue

What's perhaps more worrying is that if you tab to the 2nd pane (with
nothing in the 1st) search for something (e.g. Firefox - yeah this shouldn't
happen!) then try to close the Interface, QS just crashes.

Nice work on debugging this, I hope you can get it sorted because you're
right - it is an annoying bug!

On 17 April 2011 12:45, flyin1600 <
reply@reply.github.com>wrote:

yeah, with your and rob's help i was able to get my first build of QS up
and running in the debugger. thanks again!

i've been trying to look a little deeper into the issue described here and
i think is see what i happening. When the QSCommandBuilder window is opened
the searchObjectChanged function in QSCommandBuilder is called (i think due
to a notification being issued when setObjectValue is called when the
QSCollectingSearchObjectView is created as part of the window). That calls
the searchObjectChanged function in the super class which is
QSInterfaceController.

In QSInterfaceController's searchObjectChanged function, it checks to see
why it's getting the notification. It should be because of a change in one
of the three parts of the comman: dSelector (direct target), aSelector
(action), or indirect target (iSelector).

To do that it checks to see if the notification's object (instance of
QSCollectingSearchObjectView) is equal either dSelector, aSelector, or
iSelector. In this case the object contained by the notification is nil
because the window is being created and nothing has been populated yet.
Because the window is just being opened and hasn't been populated with
anything, that actually makes the nil object equal to dSelector, so it
enters the if case that is intended to match only if we have a valid direct
target to act on. in that if check the aSelector text box is prematurely
enabled.

that's the cause of the strange behavior inside this window. I'm pretty
sure it's the cause of the original problem described by this issue even
though it's not happening exactly the same with this new version.

i believe this could be fixed with a simple check in
QSInterfaceController:searchObjectChanged (line 363) to verify that the
object inside the notification is not nil
[[notif object] objectValue]

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

[flyin1600]

i think you're right that the intention is for the searchObjectChanged method should only be called when there is a change to the object, but i put a bunch of logs in and a breakpoint in QSInterfaceController:updateActionsNow to only be tripped if it is called for the QSCommandBuilder

if(self.windowNibName == @"CommandBuilder")
    NSLog(@"updating command builder");

i put the breakpoint at the NSLog statement, then looked at the stack trace for what is happening.

1. clicking the "+" button results in a call to QSTriggersPrefPane addTrigger:
2. then, editTriggerCommand
3. in editTriggerCommand, setCommand is called on the PrefPane's instance of QSCommandBuilder
4. setCommand calls setObjectValue to set the QSCommandBuilder's dSelector, aSelector, and i Selector to the values in the QSCommand
5. each of those setObjectValue commands results in a notification being posted for "SearchObjectChanged" One each for dSelector, aSelector, and iSelector

when the QSCommandBuilder receives the notification for dSelector, searchObjectChanged it called, which calls updateActions, which enables the aSelector box.

i'm still looking into it. i'll check out your suggestion for ignoring the tab/click action, but if we could eliminate the enable of the action box altogether with wouldn't have to ignore the tab/click. if i can get either fix to work i'll let you know.

[flyin1600]

ok so i was wrong about the reason that the aSelector box is coming up as enabled. i added some code to the searchObjectChanged function in QSInterfaceController so it does not call updateActions unless dSelector is not (null) and the aSelector box is still appearing as enabled when the CommandBuilder window is opened some reason.

there is only one place in the whole project where i see aSelector being set to enabled which is in updateActions.

back to the drawing board...

[pjrobertson]

I think it's worth doing the check now JUST for the command builder .nib

e.g. if you open the normal command window. Make sure pane 1 is empty, tab
to the 2nd pane you get exactly the same problem. What's worse is that if
you click out of QS/press escape with an item in the 2nd pane QS crashes.

The only other place I've seen things being called about actions is in
QSCommand.m:252
I stuck an NSLog in there but it doesn't look like it's getting called that
much. I'm stumped!

Good luck in figuring this out ;)
P.S. - as you go, feel free to comment/document what does what in QS. It'll
help us all a lot in the future.

On 18 April 2011 05:22, flyin1600 <
reply@reply.github.com>wrote:

ok so i was wrong about the reason that the aSelector box is coming up as
enabled. i added some code to the searchObjectChanged function in
QSInterfaceController so it does not call updateActions unless dSelector is
not (null) and the aSelector box is still appearing as enabled when the
CommandBuilder window is opened some reason.

there is only one place in the whole project where i see aSelector being
set to enabled which is in updateActions.

back to the drawing board...

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

[flyin1600]

wow i thought i was starting with something easy! i really appreciate your support with this. good point about the commenting, i'll keep that in mind as i go.

[flyin1600]

it took me a little while to fully understand your comment from yesterday. i was thinking about this problem at work today and it just popped into my head that you were trying to tell me the problem wasn't isolated to creating a trigger (i'm still new).

i found the function that handles the tab event. it's handles the tab event in both the main command window (eg: QSBezelInterfaceController) and the trigger creation window i've been looking at, CommandBuilder. It's the function called insertTab in QSSearchObjectView.m:1402

right now i'm trying to follow your suggestion and look at the action of switching to the CommandBuilder's action pane with or without a direct object selected.

if i get this fixed by modifying the CommandBuilder nib and associated code, i'm guessing all of the different interfaces for QS would have to implement a similar fix to stop that behavior from happening in their command window. I am far from familiar enough with this code base to try making a change that has such a big impact on QS as a whole, that's why i picked a bug that seemed isolated to creating a lowly trigger. when i get something working in the CommandBuilder i'll let you know then maybe we'll have a better idea for a bigger fix.

[flyin1600]

so here's what i've been able to dig up...prefaced by the fact that i'm really not as familiar with the OSX app concepts as i am the IOS app concepts...

the insertTab function i referenced above calls selectNextKeyView on the aSelector (instance of QSSearchObjectView)'s window reference.

It took me a while to figure out where all of those things were coming from because i only had the IOS developer documentation installed...i realized i was probably missing all of the OSX stuff so i reinstalled XCode and whoa...

that window is the base NSWindow for the CommandBuilder, in the insertTab method, is a line

[[self window] selectNextKeyView:self]

this is where my Cocoa knowledge gets a little hazy because i had no idea what this key concept was. i think after doing a little reading, we can make the aSelector QSObjectSearchview subscribe to a notification called NSWindowDidBecomeKeyNotification, the documenation say this is called by the becomeKeyWindow method of the NSWindow.

does this line up with what you were thinking? we add a subscription to this notification and check whether the direct object is empty? if it is empty, if it's empty change the active item back to the direct object pane?

[flyin1600]

nope nevermind

[flyin1600]

ok, what do you think...? i couldn't find a way to figure out at the level of the nib or aSelector if they became active/selected. Maybe this change to the insertTab function accomplishes the intention of your change though

QSSearchObjectView.m:1402

- (void)insertTab:(id)sender {
    [resultTimer invalidate];
    if (!([self objectValue] == nil)) {
        [[self window] selectNextKeyView:self];
    }
}

if the tab button is pressed, but you have not selected anything to be the object of your command or action, it does not pass the tab keypress on, so you are effectively "trapped" in the dSelector until you pick something to perform an action on. then once an action appears in the aSelector window you can tab over to it.

this fixes both the BezelInterface window and the CommandBuilder window

[pjrobertson]

Sounds good!

Does this fix if you 'click' in the 2nd pane though? That might be another
problem... :P

On 19 April 2011 13:10, flyin1600 <
reply@reply.github.com>wrote:

ok, what do you think...? i couldn't find a way to figure out at the level
of the nib or aSelector if they became active/selected. Maybe this change
to the insertTab function accomplishes the intention of your change though

QSSearchObjectView.m:1402

• (void)insertTab:(id)sender {
  [resultTimer invalidate];
  if (!([self objectValue] == nil)) {
  [[self window] selectNextKeyView:self];
  }
  }

if the tab button is pressed, but you have not selected anything to be the
object of your command or action, it does not pass the tab keypress on, so
you are effectively "trapped" in the dSelector until you pick something to
perform an action on. then once an action appears in the aSelector window
you can tab over to it.

this fixes both the BezelInterface window and the CommandBuilder window

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

[flyin1600]

I didn't get a chance to check before I ran out the for for work but I'm guessing no. I'll take a look at it tonight. Thanks.

[pjrobertson]

Cool cool. Your first QS bug fix is certainly going to be better than mine
was about 3 years ago - I updated a URL to point to the Google Groups!

On 19 April 2011 22:14, flyin1600 <
reply@reply.github.com>wrote:

I didn't get a chance to check before I ran out the for for work but I'm
guessing no. I'll take a look at it tonight. Thanks.

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

[flyin1600]

So I spent all night trying to get the action panel not to be active if you click on it. I'm baffled. I really think you should be able to call setEnabled:NO on the aSelector. It is a QSObjectSearchView which inherits from QSObjectView which inherits from NSControl. NSControl's documentation says that setEnabled:NO should make the NSContol not mouse click-able. Because they are added to the nib via a custom view they don't have te Inspector attributes of an NSContol, but even setting it programatically doesn't work. I put a version of setEnabled in the QSSearchObjectView which logs th call and calls [super setEnabled:passedInValue] and never saw a call to the function with YES but the action pane is still clickable...

Confused...

[flyin1600]

I also tred to steal the mouseDown message in the QSObjectSearchView and stop it from being passed up the inheritance chain with no success. The action pane would end up selected anyway.

[pjrobertson]

Hmmm.... you've been debugging this pretty well I see!

I'm no expert on user interfaces or anything with .nibs to be honest.
I'll have a think about it in the morning.

Perhaps a solution would be to just move the focus back to the 1st pane if
the 2nd pane is clicked (simulate a tab or something?) instead of actually
stopping the clicking. Your solution would be more user friendly, and it
should be possible though.

On 21 April 2011 00:09, flyin1600 <
reply@reply.github.com>wrote:

I also tred to steal the mouseDown message in the QSObjectSearchView and
stop it from being passed up the inheritance chain with no success. The
action pane would end up selected anyway.

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

[flyin1600]

well...i spent a few more days trying to figure out how to stop the action panel from being clickable and i'm stumped. most recently i tried to use the same code that the indirect object panel uses to make itself disabled but it actually removes itself from the view entirely and that causes the main window to have major problems opening.

i was looking for some kind of "didBecomeActive" method like you suggested but i haven't been able to find one.

i have a way to make text not show up in the action pane if you have nothing in the direct object pane. i added an if-check to the insertText method of the NSText piece of the control that check to see if the currently QSSearchObjectView is the aSelector and if there is no text in the direct object pane, if both of those are true it doesn't allow the text, if not then you pass.

QSSearchObjectView.m:1474

- (void)insertText:(id)aString {
    //aString = [[aString purifiedString] lowercaseString];
    if(!(([[self controller] aSelector] == self) && ([[[self controller] dSelector] currentEditor] == nil))){
        if (![partialString length]) {
            [self updateHistory];
            [self setSearchArray:sourceArray];
        }
        [partialString appendString:aString];
        [self partialStringChanged];
    }
}

while definitely not the optimal solution...i think the combination of the above change to disallow tabbing from the direct object pane, and disallowing writing in the action pane when the direct object is empty (in case someone clicks on the action pane) accomplishes the fix.

what do you think?

[pjrobertson]

It sounds good, and it's covering most options. I know what it's like when
things just won't work!

One last suggestion; have you tried looking at mouseDown
in QSSearchObjectView.m:1172
I don't know much about NSView and Interface Builder things, so I'm not
going to be the best person to say what's what, but you could try looking in
the Documentation at the NSView stuff.
One thing I didn't do when I first started coding was use the Documentation.
The sooner you start using it the better! (I'm not sure if it's the case for
you)

It seems like @bencochran knows the most about interfaces etc. (as he's the
one making BezelHUD now) so he may have some pointers. If not, then I'd be
happy with this suggestion :)

On 24 April 2011 01:08, flyin1600 <
reply@reply.github.com>wrote:

well...i spent a few more days trying to figure out how to stop the action
panel from being clickable and i'm stumped. most recently i tried to use
the same code that the indirect object panel uses to make itself disabled
but it actually removes itself from the view entirely and that causes the
main window to have major problems opening.

i was looking for some kind of "didBecomeActive" method like you suggested
but i haven't been able to find one.

i have a way to make text not show up in the action pane if you have
nothing in the direct object pane. i added an if-check to the insertText
method of the NSText piece of the control that check to see if the currently
QSSearchObjectView is the aSelector and if there is no text in the direct
object pane, if both of those are true it doesn't allow the text, if not
then you pass.

QSSearchObjectView.m:1474

• (void)insertText:(id)aString {
  //aString = [[aString purifiedString] lowercaseString];
  if(!(([[self controller] aSelector] == self) && ([[[self controller]
  dSelector] currentEditor] == nil))){
  if (![partialString length]) {
  [self updateHistory];
  [self setSearchArray:sourceArray];
  }
  [partialString appendString:aString];
  [self partialStringChanged];
  }
  }

while definitely not the optimal solution...i think the combination of the
above change to disallow tabbing from the direct object pane, and
disallowing writing in the action pane when the direct object is empty (in
case someone clicks on the action pane) accomplishes the fix.

what do you think?

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

[pjrobertson]

Whilst looking at another bug, I saw the 'mouseDown' method as well and thought about doing a

 if([self objectValue] == nil)

test, and you're right - it doesn't work. QS registers there's no object in the 1st pane, but using 'return' still lets the receiver be clicked. Very strange!

Maybe someone with more NSView knowledge should have a look at this? I think we need some dev who's IB savvy on board!

[pjrobertson]

Just a thought.

Before we go down the root of thinking 'the first pane is empty so lets not leave it'

how about use this way of thinking:
'the first pane is empty so lets grab the current selection and find an action'
Yes, this isn't one of my ideas. I've just been watching Nicholas (again...)
http://video.google.com/videoplay?docid=8493378861634507068&hl=en#

at 21:10 he mentions this idea.

Basically, I think doing a check for:

 if(1st pane is empty and we've tabbed to the second pane) {
        grab the current selection and start searching for an action
        if(the current selection dObject gives nothing or can't be resolved)
              don't go to the second pane but wait
  }

[ghost]

i can't reproduce this with the latest build. i think this was fixed for issue #229.

also @pjrobertson, Alcor actually implemented something like that in QSB - actions being searchable at least, i'm not sure if every action works in both noun-verb and verb-noun orientations. there are a couple of other features he mentions at around the same time in the video which were implemented in QSB as well, like inline Google and Spotlight results, which are things which would be really cool to have in Quicksilver, but would require huge architectural changes. maybe it'd be a good idea to create a wiki page where we could discuss ideas for major modifications to Quicksilver.

it's such a great video. scoped catalogs might be worth doing as well (Alcor mentions that alongside the idea you talk about above).

[pjrobertson]

I've had an 'experimental' branch since I made that post.

It fixes this bug (I still get the problem) and also changes QS so that, when you tab with nothing in the first pane it assumes you mean the 'currently selected thing' and you can then do something with that. Similar to ⌘�⎋

[pjrobertson]

I've just gone ahead and tried to fix this, but as neurolepsy said, this was indeed fixed by #229. I can't reproduce it.

If we want to enable triggers for just actions that could be possible, but I suggest we open another ticket.
The bug where you get catalog items if you tab to the 2nd pane and type (if the 1st pane is empty) still persists, but I still have a fix for that lying around somewhere