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

Prevent docking windows (shelf and clipboard) from showing on launch #692

Merged
merged 2 commits into from Apr 7, 2012
Merged

Prevent docking windows (shelf and clipboard) from showing on launch #692

merged 2 commits into from Apr 7, 2012

Conversation

skurfer
Copy link
Member

@skurfer skurfer commented Feb 9, 2012

On my system, the value for hidden was being toggled every time Quicksilver shut down.

I noticed that there wasn’t necessarily an explicit value for (BOOL)hidden, but there are several places where behavior is determined by its value.

If hidden is initially YES, the plug-ins will still show the window based on the saved state, so this doesn’t break the state-preserving behavior. If it’s initially undefined, then it evaluates to NO (and the plug-ins do nothing to override that), which is what caused the problem.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Feb 10, 2012

Does this also let you dock the window, or is that still broken?

@skurfer
Copy link
Member Author

@skurfer skurfer commented Feb 10, 2012

Does this also let you dock the window, or is that still broken?

That one’s still a mystery. Doesn’t work in a clean account either. Could it be hardware somehow? No hurry as I won’t be able to look at it for a while, but would you mind sending me a diff of your Quicksilver preferences with the Shelf docked and undocked?

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Feb 10, 2012

That ones still a mystery. Doesnt work in a clean account either. Could
it be hardware somehow? No hurry as I wont be able to look at it for a
while, but would you mind sending me a diff of your Quicksilver preferences
with the Shelf docked and undocked?

Can do, but not right now either.

I remember we discussed the possibility of it being something to do with 2
screens. Have you tried with just one? But to be fair, I often use 2 and it
works for me. Are you on a desktop?

On 10 February 2012 18:26, Rob McBroom <
reply@reply.github.com

wrote:

Does this also let you dock the window, or is that still broken?

That ones still a mystery. Doesnt work in a clean account either. Could
it be hardware somehow? No hurry as I wont be able to look at it for a
while, but would you mind sending me a diff of your Quicksilver preferences
with the Shelf docked and undocked?


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

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Feb 10, 2012

This causes docked windows to always display on startup, not good :(

@skurfer
Copy link
Member Author

@skurfer skurfer commented Feb 10, 2012

This causes docked windows to always display on startup, not good :(

Well, I couldn’t possibly have tested that. :-) So I guess this’ll have to wait until 66. I’ll open a separate issue on the docking business when I get a chance to describe my latest findings. It could turn into a lengthy discussion and it’s not central to this pull request.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Feb 11, 2012

So I guess thisll have to wait until 66.

Seeing as how this doesn't work for me... I guess so!
I'll release 65 pre tomorrow if you don;t get in before me ;)

On 10 February 2012 19:59, Rob McBroom <
reply@reply.github.com

wrote:

This causes docked windows to always display on startup, not good :(

Well, I couldnt possibly have tested that. :-) So I guess thisll have to
wait until 66. Ill open a separate issue on the docking business when I
get a chance to describe my latest findings. It could turn into a lengthy
discussion and its not central to this pull request.


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

@skurfer
Copy link
Member Author

@skurfer skurfer commented Feb 13, 2012

Is it canFade that tells it whether or not the thing is docked? Maybe that’s what’s causing the setting to get saved incorrectly for you (causing the window to always show). With my change here merged in, I wonder if the check for canFade in the plug-ins’ saveVisibilityState: is even necessary. Can you try removing it?

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Feb 13, 2012

I had to add the canFade check into saveVisibilityState. It's all
pretty complicated...

See my comment here: quicksilver/Shelf-qsplugin#3

"The fix: On quit, we need to check if the shelf window is visible (using
hidden). If not, then we need to check if itcanFade - i.e. it's docked.

Apparently a docked shelf window is visible according to the OS, but the
hidden method doesn't say so that it's visible. i.e. when the window is
docked, canFade returns yes but hidden returns NO, but even though the
window isn't ACTUALLY hidden (it's peeping out from the side of your
window)

Confusing :)"

On 13 February 2012 14:55, Rob McBroom <
reply@reply.github.com

wrote:

Is it canFade that tells it whether or not the thing is docked? Maybe
thats whats causing the setting to get saved incorrectly for you (causing
the window to always show). With my change here merged in, I wonder if the
check for canFade in the plug-ins saveVisibilityState: is even
necessary. Can you try removing it?


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

@skurfer
Copy link
Member Author

@skurfer skurfer commented Feb 13, 2012

Confusing for sure. It seems like there are two things we care about: Is the window open? Is the window docked? (which means open but not showing)

But the setting only stores one value, and that value answers yet a third question: Should I show the window on start up?

Correct me if I’m wrong, but if the stored setting is NO, the window will not be restored at all. If the setting is YES, the window will be both open and shown on screen (whether it was docked or not). It sounds like what you’re expecting it to do is open the window, but don’t show it (keep it off screen).

Still confused, but I think the correct solution is for the plug-ins to test canFade on start-up, not on shut down. In other words, they need two values to determine correct start-up behavior, but the plist only stores one. I’ll do some testing here to confirm.

@skurfer
Copy link
Member Author

@skurfer skurfer commented Feb 13, 2012

Still think I’m on the right track, but still confused. The plug-in either calls showClipboardHidden:, causing the panel to display, or it doesn’t. There is no third “open it but keep it hidden off screen” behavior to choose from, so how did this ever work?

Does the showClipboardHidden: behave differently based on sender? And is the difference determined by the default value of (BOOL)hidden which I have attempted to change? That’s the only thing I can think of.

Sorry to bug you with so much of this. It seems that in order to get the sliding dock behavior, I have to hit ⌘L and resize the panel after every launch. (This is the case with B64 as well, so it wasn’t this change.) So I’m unable to test whether a docked panel gets restored correctly on launch with different settings since it never gets restored correctly under any circumstances. :-)

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Feb 13, 2012

I'll try and get all your questions answered ASAP but I'm bogged down at
the moment.

Sorry to bug you with so much of this. It seems that in order to get the
sliding dock behavior, I have to hit ⌘L and resize the panel after every
launch. (This is the case with B64 as well, so it wasn’t this change.) So
I’m unable to test whether a docked panel gets restored correctly on launch
with different settings since it never gets restored correctly under any
circumstances. :-)

The business of having to hit ⌘L was a bug with the previous version of the
plugin that I updated a week or so ago. Have you got the latest source?
Make sure you drag the window so a corner is over the edge of the screen,
i.e. it half hidden behind the screen.

Does the showClipboardHidden: behave differently based on sender? And
is the difference determined by the default value of (BOOL)hidden which I
have attempted to change? That’s the only thing I can think of.

No, it shouldn't do. See orderFrontHidden: in QSDockingWindow.m (in the
QS folder)
If the window canFade (i.e. it is on the screen edge) it really bring the
window to the front and really orders it to the front
(QSDockingWindow.m:190) by making it pop up, otherwise, if it can't fade
(canFade returns NO) then it's floating and is just brought front.

Correct me if I’m wrong, but if the stored setting is NO, the window will
not be restored at all. If the setting is YES, the window will be both open
and shown on screen (whether it was docked or not). It sounds like what
you’re expecting it to do is open the window, but don’t show it (keep it
off screen).

If the value is YES then the window should show wherever it is on the
screen. If it's NO, then it'll be hidden. QS decides whether or not the
window is docked or not (canFade) based on whether the window's rect
overlaps the screen rect. I think this is where the bug comes in with
multiple screens.

On 13 February 2012 20:00, Rob McBroom <
reply@reply.github.com

wrote:

Still think I’m on the right track, but still confused. The plug-in either
calls showClipboardHidden:, causing the panel to display, or it doesn’t.
There is no third “open it but keep it hidden off screen” behavior to
choose from, so how did this ever work?

Does the showClipboardHidden: behave differently based on sender? And
is the difference determined by the default value of (BOOL)hidden which I
have attempted to change? That’s the only thing I can think of.

Sorry to bug you with so much of this. It seems that in order to get the
sliding dock behavior, I have to hit ⌘L and resize the panel after every
launch. (This is the case with B64 as well, so it wasn’t this change.) So
I’m unable to test whether a docked panel gets restored correctly on launch
with different settings since it never gets restored correctly under any
circumstances. :-)


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

@HenningJ
Copy link
Contributor

@HenningJ HenningJ commented Feb 14, 2012

QS decides whether or not the window is docked or not (canFade) based on whether the window's rect overlaps the screen rect. I think this is where the bug comes in with multiple screens.

I think so, too.
@skurfer can you debug the canFade method and figure out what [[self screen] frame] returns when you have two displays and when you have only one?
Btw. how are your two screens set up and where do you try to dock the window?

@skurfer
Copy link
Member Author

@skurfer skurfer commented Feb 15, 2012

Crap. I posted a long reply here yesterday and it’s not here. Trying again.

I think I know why you didn’t know about this bug. If I dock the panel, it never shows on startup. Using a build without this change, try moving one of your panels so it floats. Close it, then restart a few times. Does it appear on every other launch? Or just run defaults read com.blacktree.Quicksilver QSPasteboardHistoryIsVisible and watch it flip on and off. :-)

Skipping a lot of detail, but I was right about being able to fix Patrick’s complaint by altering the plug-in to determine startup behavior at startup instead of at shutdown, but there’s still a problem. Now docked windows don’t work on startup.

I think in order to work, they need to be opened then immediately hidden. This change prevents them from being hidden, as Patrick pointed out. (The first line of hide: says if (hidden) return;.) My change to the plug-in (to address that problem) prevents them from opening at all. So I’m now thinking this can be fixed without altering the plug-ins, so I’ll look into it and send the commits when I’ve figured it out.

can you debug the canFade method and figure out what [[self screen] frame] returns when you have two displays and when you have only one?

See #696 - I think multiple displays have been eliminated as a problem. Like I said on that issue, I can get some limited docking to work, and in that case, canFade returns the correct value.

@skurfer
Copy link
Member Author

@skurfer skurfer commented Feb 15, 2012

Figured out the problem in my head while I was at lunch (but not the solution yet).

On launch, hidden is undefined, so for practical purposes, it’s NO.

If the preference to show the panel on startup is YES, the panel is shown. The user closes it, which sets hidden = YES. On shutdown, if hidden == YES, the preference is written as NO.

If the preference to show the panel on startup is NO, the panel is never shown or touched leaving hidden as it’s default value of NO. On shutdown, if hidden == NO, the preference is written as YES.

So it flips every time the application runs.

@skurfer
Copy link
Member Author

@skurfer skurfer commented Feb 16, 2012

Another one-line change, and now I believe the state is always restored correctly for clipboard and shelf panels on startup.

An even simpler explanation of the original problem: If a panel is never displayed through the life of the application, it’s obviously “hidden”, but its hidden property is never set to anything so it evaluates to NO. The first commit fixes that by defaulting to hidden = YES.

In the second commit, I’ve just removed what appears to be an inappropriate optimization. The hide: method obviously does a lot more than alter the value of hidden, so that alone is not a good test to see if the method should run. I was concerned about repeated calls to this method, but in debugging, it seems that it only gets called when the “hide off screen” truly needs to take place. I didn’t see any scenarios where the method would be called unnecessarily, so there was no benefit to returning from the method early.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Apr 4, 2012

Strangely enough, this seems to build, but why is BOOL hidden commented out in QSDockingWindow.h?
Seems like the iVar's class/type is never set anywhere at the moment, so in theory the hidden = YES line you added in your first commit could be setting YES to anything...

actually, option clicking the word hidden in Xcode shows that Xcode things it's some kind of NSLevelIndicator (binding) which certainly isn't what it is!

You'll notice that if you uncomment the BOOL hidden line in the header file, you get all sorts of strange errors when trying to build. I think that's because it's an already defined value/keyword. It's like trying to do this in the header NSString *self

My suggestion is that you change the BOOL name to something like isHidden or hiddenValue in the QSDocking window. If the -(BOOL)hidden; method keeps the same name, then no other classes calling this should have a problem.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Apr 4, 2012

Take a look at 6edfcd1 for what I did to get things building properly. Feel free to cherry-pick that commit

@skurfer
Copy link
Member Author

@skurfer skurfer commented Apr 4, 2012

actually, option clicking the word hidden in Xcode shows that Xcode things it's some kind of NSLevelIndicator (binding) which certainly isn't what it is!

I Command clicked it to see where it’s defined. It appears to be an iVar for QSWindow, which QSDockingWindow inherits from (by way of QSBorderlessWindow).

I don’t know for sure, but I don’t think the bindings stuff applies here. That is, I don’t think “hidden” is some sort of reserved keyword. But if it is wrong to use that, it needs to be fixed from QSWindow on down.

pjrobertson added a commit that referenced this issue Apr 7, 2012
Prevent docking windows (shelf and clipboard) from showing on launch
@pjrobertson pjrobertson merged commit 4b253d7 into quicksilver:master Apr 7, 2012
@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented May 1, 2012

I'm not sure if it's this pull request that caused it, but I've seen the following exception twice in so many days:

01/05/2012 12:25:54.490 Quicksilver: An uncaught exception was raised
01/05/2012 12:25:54.491 Quicksilver: -[NSKeyValueObservance target]: unrecognized selector sent to instance 0x34a700


01/05/2012 12:25:54.493 Quicksilver: (
    0   CoreFoundation                      0x9565da67 __raiseError + 231
    1   libobjc.A.dylib                     0x930b5149 objc_exception_throw + 155
    2   ExceptionHandling                   0x001f0f08 -[NSExceptionHandler _handleException:mask:] + 1422
    3   ExceptionHandling                   0x001f114d NSExceptionHandlerExceptionRaiser + 232
    4   libobjc.A.dylib                     0x930b5149 objc_exception_throw + 155
    5   CoreFoundation                      0x95661070 -[NSObject doesNotRecognizeSelector:] + 256
    6   CoreFoundation                      0x955afcd9 ___forwarding___ + 457
    7   CoreFoundation                      0x955afaa2 _CF_forwarding_prep_0 + 50
    8   QSEffects                           0x000cea11 -[QSMoveHelper _doAnimation] + 321
    9   QSEffects                           0x000ce4b5 -[QSAnimationHelper _threadAnimation] + 165
    10  QSEffects                           0x000ced1f -[QSMoveHelper _resizeWindow:toFrame:alpha:display:] + 447
    11  QSInterface                         0x0018ee0f -[QSDockingWindow hide:] + 1839
    12  QSInterface                         0x0018e57e -[QSDockingWindow hideOrOrderOut:] + 94
    13  Foundation                          0x9a7d5df1 __-[NSNotificationCenter addObserver:selector:name:object:]_block_invoke_1 + 49
    14  CoreFoundation                      0x9559d903 ___CFXNotificationPost_block_invoke_1 + 275
    15  CoreFoundation                      0x95568688 _CFXNotificationPost + 2776
    16  Foundation                          0x9a7c0fde -[NSNotificationCenter postNotificationName:object:userInfo:] + 92
    17  QSCore                              0x00128a77 appChanged + 247
    18  HIToolbox                           0x918b4dec _Z22_InvokeEventHandlerUPPP25OpaqueEventHandlerCallRefP14OpaqueEventRefPvPFlS0_S2_S3_E + 36
    19  HIToolbox                           0x917304f3 _ZL23DispatchEventToHandlersP14EventTargetRecP14OpaqueEventRefP14HandlerCallRec + 1602
    20  HIToolbox                           0x9172f970 _ZL30SendEventToEventTargetInternalP14OpaqueEventRefP20OpaqueEventTargetRefP14HandlerCallRec + 482
    21  HIToolbox                           0x9172f788 SendEventToEventTargetWithOptions + 75
    22  HIToolbox                           0x91772f62 HIToolboxLSNotificationCallbackAllASNsFunc + 559
    23  LaunchServices                      0x91bd6966 ___LSScheduleNotificationFunction_block_invoke_2 + 72
    24  LaunchServices                      0x91bd6566 _LSCallbackNotificationCallback + 355
    25  LaunchServices                      0x91bd63f8 _XNotificationCallback + 476
    26  LaunchServices                      0x91bd6211 LSAPluginCallbacks_server + 155
    27  LaunchServices                      0x91bd60f0 _ZL38LSNotificationHandleCallbackFromServerP17mach_msg_header_t + 38
    28  CoreFoundation                      0x9551ad0a __CFMachPortPerform + 346
    29  CoreFoundation                      0x9551ab91 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE1_PERFORM_FUNCTION__ + 49
    30  CoreFoundation                      0x9551a7bb __CFRunLoopDoSource1 + 155
    31  CoreFoundation                      0x95553e01 __CFRunLoopRun + 2193
    32  CoreFoundation                      0x955531dc CFRunLoopRunSpecific + 332
    33  CoreFoundation                      0x95553088 CFRunLoopRunInMode + 120
    34  HIToolbox                           0x9172a723 RunCurrentEventLoopInMode + 318
    35  HIToolbox                           0x91731a8b ReceiveNextEventCommon + 381
    36  HIToolbox                           0x917318fa BlockUntilNextEventMatchingListInMode + 88
    37  AppKit                              0x99c5515c _DPSNextEvent + 678
    38  AppKit                              0x99c549c6 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 113
    39  AppKit                              0x99c50d35 -[NSApplication run] + 911
    40  AppKit                              0x99ee1c59 NSApplicationMain + 1054
    41  Quicksilver                         0x000219c5 main + 309
    42  Quicksilver                         0x00002515 start + 53
)

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented May 1, 2012

Although, the 2nd time it happened, I was running QS from /tmp/QS/Debug AND building a different debug build in QS, so that could have messed things up

@skurfer
Copy link
Member Author

@skurfer skurfer commented May 2, 2012

I haven’t seen this exception myself. In any case, I’d try running with #824 merged in to see if it changes anything.

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 this pull request may close these issues.

None yet

3 participants