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

Perform keyboard switching on main thread. #1865

Merged
merged 1 commit into from Jun 4, 2014
Merged

Conversation

pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jun 3, 2014

Nothing except https://developer.apple.com/library/mac/technotes/tn2125/_index.html specifies that HIToolbox can't be run on the main thread, but switching keyboards seems like something that should.

I meant to put this in with my other 'fixes' branch, but seems like I forgot.
From git blame I see that @tiennou changed the keyboard switching code to run on a background thread. Can I ask why?

I'm still getting crashes when activating QS, and I think it's because of this

Nothing except https://developer.apple.com/library/mac/technotes/tn2125/_index.html specifies that HIToolbox can't be run on the main thread, but switching keyboards seems like something that should.
@tiennou
Copy link
Member

tiennou commented Jun 3, 2014

Good question. I don't think I had any specific reason to actually :-S. I think you're right (unless running with the fix still crashes that is ;-)), I don't see anything that clearly states that TIS is threadsafe, so better be safe than sorry and make it main-thread only.

@pjrobertson
Copy link
Member Author

pjrobertson commented Jun 4, 2014

Cool, thanks for the response :)

@skurfer skurfer added this to the 1.2.0 milestone Jun 4, 2014
skurfer added a commit that referenced this pull request Jun 4, 2014
Perform keyboard switching on main thread.
@skurfer skurfer merged commit 7457748 into master Jun 4, 2014
@skurfer skurfer deleted the moreKeyboardChanges branch Jun 4, 2014
@skurfer
Copy link
Member

skurfer commented Jun 4, 2014

Does this fix #1850?

@pjrobertson
Copy link
Member Author

pjrobertson commented Jun 4, 2014

Nope, unfortunately not.

Here's the scenario where I could reproduce #1850:

  • I have a trigger set up for say ⌘A
  • I launch QS with QWERTY (UK English) as my keyboard, and the trigger's registered
  • I switch to Hangul (Korean) keyboard, and the triggers are re-scoped
  • QS can't find the 'A' character on the keyboard, so the 'enable [of the hotkey] failed'

This is the closest I got to fixing it

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