cocoabits / MASShortcut

Modern framework for managing global keyboard shortcuts compatible with Mac App Store. More details:
http://blog.shpakovski.com/2012/07/global-keyboard-shortcuts-in-cocoa.html
BSD 2-Clause "Simplified" License
1.52k stars 220 forks source link

Only seeing XXX in UI for MASShortcutView #74

Closed beaufour closed 8 years ago

beaufour commented 9 years ago

I am using MASShortcut v2.3.1 via CocoaPods. When I insert a MASShortcutView into my UI it works fine, but all text is shown as XXX in the UI. I suspect that Localizable.strings is not part of the pod?

I can fix it by manually copying the strings from the MASShortcut source into Localizable.strings in my own project, but hopefully that shouldn't be necessary :smile:

Great library btw!

shpakovski commented 9 years ago

Indeed, this is a problem, thank you very much for the report! @zoul Will it help if we rename the Localizable.strings and use a custom table name?

zoul commented 9 years ago

I currently use the localized MASShortcut in a production app and it works correctly, but I use Carthage, not CocoaPods. So I think the problem is in our CocoaPods manifest, probably our Localized.strings file is not installed at all. I’ll take a look at it, just don’t know when yet.

shpakovski commented 9 years ago

I think this will not work, because MASShortcuts version would just rewrite the local project Localizable.strings file, there is no “merge policy” for Pods.

zoul commented 9 years ago

Oh, I see – this is why I find Carthage better, it just sticks to the framework that’s already defined. Judging by this blog post and this Stack Overflow thread, it looks like we need to create a resource bundle to stick the strings file into?

shpakovski commented 9 years ago

Yeah… This is not very convenient for such a small number of strings :) Any better idea? @beaufour?

beaufour commented 9 years ago

I really wish I knew more about the inner workings of CocoaPods, but alas I don't.

beaufour commented 9 years ago

Unless I've missed something, it seems like it wasn't too much work :)

shpakovski commented 9 years ago

Thanks!

zoul commented 9 years ago

This is now available in the 2.3.2 release on CocoaPods. @beaufour, can you please verify that the fix works in the version distributed by CocoaPods?

beaufour commented 9 years ago

:+1: Works like a charm.

shpakovski commented 9 years ago

Thank you guys 👻

zoul commented 9 years ago

:+1:

jhoughjr commented 9 years ago

This might be related as I'm having reports of users getting some hangs. I'm using 2.3.2 as a cocoapod.

the crash is : 10/21/15 11:50:25.617 AM com.apple.xpc.launchd[1]: (com.apple.xpc.launchd.oneshot.0x1000002b.GhostNote[19341]) Service exited due to signal: Killed: 9 10/21/15 11:51:20.967 AM GhostNote[24081]: * -[NSKeyedUnarchiver initForReadingWithData:]: data is NULL 10/21/15 11:52:15.079 AM GhostNote[24081]: * -[NSBundle initWithURL:]: nil URL argument 10/21/15 11:52:15.081 AM GhostNote[24081]: ( 0 CoreFoundation 0x00007fff9e4a2bd2 exceptionPreprocess + 178 1 libobjc.A.dylib 0x00007fff8b8b9dd4 objc_exception_throw + 48 2 CoreFoundation 0x00007fff9e5093fd +[NSException raise:format:] + 205 3 Foundation 0x00007fff9cd84ea2 -[NSBundle initWithURL:] + 87 4 Foundation 0x00007fff9cd84e38 +[NSBundle bundleWithURL:] + 45 5 MASShortcut 0x0000000109875f64 MASLocalizedString + 132 6 MASShortcut 0x0000000109879951 -[MASShortcutView drawRect:] + 989 7 AppKit 0x00007fff8c750d5e -[NSView _drawRect:clip:] + 3626 8 AppKit 0x00007fff8c7a951d -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 1873 9 AppKit 0x00007fff8c7a98fa -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 2862 10 AppKit 0x00007fff8c7a98fa -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 2862 11 AppKit 0x00007fff8c74e487 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 838 12 AppKit 0x00007fff8c74dc6c -[NSThemeFrame _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 334 13 AppKit 0x00007fff8c74c077 -[NSView _displayRectIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:] + 2449 14 AppKit 0x00007fff8c747485 -[NSView displayIfNeeded] + 1950 15 AppKit 0x00007fff8c746ccc -[NSWindow displayIfNeeded] + 232 16 AppKit 0x00007fff8cdc5206 _NSWindowGetDisplayCycleObserver_block_invoke6323 + 476 17 AppKit 0x00007fff8c746667 __37+[NSDisplayCycle currentDisplayCycle]_block_invoke + 738 18 QuartzCore 0x00007fff9e96b91d _ZN2CA11Transaction19run_commit_handlersE18CATransactionPhase + 85 19 QuartzCore 0x00007fff9e96add8 _ZN2CA7Context18commit_transactionEPNS_11TransactionE + 160 20 QuartzCore 0x00007fff9e96aa98 _ZN2CA11Transaction6commitEv + 508 21 QuartzCore 0x00007fff9e97628f _ZN2CA11Transaction17observer_callbackEP19CFRunLoopObservermPv + 71 22 CoreFoundation 0x00007fff9e437e07 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION + 23 23 CoreFoundation 0x00007fff9e437d77 __CFRunLoopDoObservers + 391 24 CoreFoundation 0x00007fff9e416d58 CFRunLoopRunSpecific + 328 25 HIToolbox 0x00007fff90f35d55 RunCurrentEventLoopInMode + 235 26 HIToolbox 0x00007fff90f35a97 ReceiveNextEventCommon + 184 27 HIToolbox 0x00007fff90f359cf _BlockUntilNextEventMatchingListInModeWithFilter + 71 28 AppKit 0x00007fff8c5eef3a _DPSNextEvent + 1067 29 AppKit 0x00007fff8c5ee369 -[NSApplication _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 454 30 AppKit 0x00007fff8c5e2ecc -[NSApplication run] + 682 31 GhostNote 0x00000001097e76dd GhostNote + 108253 32 libdyld.dylib 0x00007fff8d6cb5ad start + 1

jhoughjr commented 9 years ago

at a breakpoint on line 12 of MASLocalization.m bundleURL is nil.

I'm not sure why I don't get this crash and other users do, but perhaps it has to do with their locale.

I just checked and it appears that the bundle it refers to is in the built product inside the MASShortcut.framework that cocoa pods generated.

shpakovski commented 9 years ago

Thanks a lot for the problem report!

To me it looks like this code may help:

NSString *MASLocalizedString(NSString *key, NSString *comment) {
    NSBundle *frameworkBundle = [NSBundle bundleForClass:[MASShortcut class]];
#ifdef COCOAPODS
    NSURL *bundleURL = [frameworkBundle URLForResource:@"MASShortcut" withExtension:@"bundle"];
    frameworkBundle = [NSBundle bundleWithURL:bundleURL];
#endif
    return [frameworkBundle localizedStringForKey:key value:@"XXX" table:@"Localizable"];
}

Can you check if it works please?

jhoughjr commented 9 years ago

I believe that is the code that exists in the current latest version. I found this issue since some users are getting a blocked UI when showing the view that contains the MASShortcutView. For some reason though, for me it does not hang, but I would see the exception in the debugger.

The URL was nil, and this code seems to resolve properly and get the bundle.

NSString MASLocalizedString(NSString key, NSString comment) { NSBundle frameworkBundle = nil;

ifdef COCOAPODS

// NSURL bundleURL = [[NSBundle mainBundle] URLForResource:@"MASShortcut" withExtension:@"framework"]; //
NSURL * bundleURL = [[NSBundle mainBundle] bundleURL]; NSURL
frameworkBundleURL = [bundleURL URLByAppendingPathComponent:@"Contents/Frameworks/MASShortcut.framework/Versions/A/Resources/MASShortcut.bundle"];

frameworkBundle = [NSBundle bundleWithURL:frameworkBundleURL];

else

frameworkBundle = [NSBundle bundleForClass:[MASShortcut class]];

endif

return [frameworkBundle localizedStringForKey:key value:@"XXX" table:@"Localizable"];

}

My pod file has the use_frameworks! directive, perhaps that is the difference. Though that code appears it wouldn’t return a value if COCOAPODS is defined?

On Oct 22, 2015, at 7:25 AM, Vadim Shpakovski notifications@github.com wrote:

Thanks a lot for the problem report!

To me it looks like this code may help:

NSString MASLocalizedString(NSString key, NSString comment) { NSBundle frameworkBundle = [NSBundle bundleForClass:[MASShortcut class]];

ifdef COCOAPODS

NSURL *bundleURL = [frameworkBundle URLForResource:@"MASShortcut" withExtension:@"bundle"];
frameworkBundle = [NSBundle bundleWithURL:bundleURL];

else

return [frameworkBundle localizedStringForKey:key value:@"XXX" table:@"Localizable"];

} Can you check if it works please?

— Reply to this email directly or view it on GitHub https://github.com/shpakovski/MASShortcut/issues/74#issuecomment-150204575.

beaufour commented 9 years ago

@jhoughjr so you are seeing the bundleURL == nil in your local build? And you build with use_frameworks!? (just making sure I fully understand it)

jhoughjr commented 9 years ago

Yes I think that is what is happening. Currently in specifying a hard coded url gleaned from inspecting the app bundle and the issue seems fixed.

Sent from my iPhone

On Oct 26, 2015, at 3:31 PM, Allan Beaufour notifications@github.com wrote:

@jhoughjr so you are seeing the bundleURL == nil in your local build? And you build with use_frameworks!? (just making sure I fully understand it)

— Reply to this email directly or view it on GitHub.

beaufour commented 9 years ago

Yes I think that is what is happening. Currently in specifying a hard coded url gleaned from inspecting the app bundle and the issue seems fixed.

Building as a framework is what Carthage is doing as far as I know, so the trick might just to only do the COCOAPODS ugliness when using CocoaPods but not when building as a framework. If you force the code to take the #else branch does it work for you?

kujenga commented 9 years ago

I just ran a pod update on my application and am now running into the same issue that @jhoughjr was seeing. I also am using the use_frameworks! flag in my Podfile. The previous version I was using was 2.2.0, which worked just fine for me. For now I've just pinned to that specific version in my Podfile to keep things working, but that isn't the greatest solution.

zoul commented 9 years ago

Did we find out whether the use_frameworks! flag makes COCOAPODS false? That would explain the issue.

beaufour commented 9 years ago

Did we find out whether the use_frameworks! flag makes COCOAPODS false? That would explain the issue.

My guess is that it actually makes it true, but that use_frameworks doesn't build the bundle so it evaluates to nil. But as said, I'm guessing. I have never used use_frameworks myself.

kujenga commented 9 years ago

I think that COCOAPODS is always set to true when pod install is run as part of the configuration performed by CocoaPods, regardless of whether you are using frameworks in your application. In looking at my build settings for the application targets, I see that it has been specifically set in both the "Preprocessor Macros" section for Obj-C and the "Other Swift Flags" section for Swift (my application is Swift based and contains Objective-C code).

In the Cocoapods documentation [1] on using frameworks it seems to suggest that the original use of [NSBundle bundleForClass:[MASShortcut class]] pre https://github.com/shpakovski/MASShortcut/pull/75 should work fine, as it specifically says, "This will then work for both frameworks and static libraries". However, when I manually modify MASLocalization.m to this effect, I run into the original issue @beaufour was seeing with the XXX fallback being displayed everywhere.

[1] http://blog.cocoapods.org/CocoaPods-0.36/#caveats

zoul commented 9 years ago

I have tried to fix the problem by not guessing which bundle to use and simply using the one that works. Can you all please test the new code if it really works as intended? Sorry for the trouble.

beaufour commented 9 years ago

:+1: Works for me (not using use_frameworks!). Nice dispatch_once pattern.

kujenga commented 9 years ago

Sorry for the relayed response. This code does not crash for me, with use_frameworks! turned on, but unfortunately I'm back to the old issue of the XXX default value being shown everywhere.

I'll poke around to see if I can get something working.

zoul commented 9 years ago

So how about now? The current code tests if the CocaPods bundle contains the localized strings. If not, it switches to the framework bundle instead.

jhoughjr commented 9 years ago

Sorry everyone, i was very busy the last couple of weeks wrapping up a contract. I'll test the latest version and let you know tonight.

olivierguerriat commented 8 years ago

Another data point: 7fc5e78 works for me (with use_frameworks!). Thanks for the fix!

zoul commented 8 years ago

This should be now fixed by d32fc18 (shipped as a part of 2.3.3), sorry for the long wait.

madrobby commented 8 years ago

I'm on Cocoapods version of MASShortcut v2.3.3 in a RubyMotion project (I'm on RubyMotion v4.12). I see the "XXX" instead of real strings.

  1. Has anybody else experienced this with RubyMotion?
  2. "XXX" is an extremely bad placeholder because you can't google for it (try it, it's NSFW)