sparkle-project / Sparkle

A software update framework for macOS
https://sparkle-project.org
Other
7.46k stars 1.05k forks source link

Catalyst Release Notes Support #1482

Closed Vortec4800 closed 1 year ago

Vortec4800 commented 5 years ago

Can I use Sparkle to auto-update apps (distributed outside of the Mac App Store of course) that are built using Catalyst? Is there anything I need to do special to make it all work?

kornelski commented 5 years ago

I haven't tried Catalyst. If it's compatible with Cocoa and can use frameworks, it should be able to use Sparkle.

samdeane commented 4 years ago

Sparkle builds against the normal macOS sdk, so I'm pretty sure you can't use it directly from a Catalyst app.

You can probably use it by bridging to macOS via a plugin, and having the plugin link against Sparkle and create the SUUpdater object.

I'm not sure how well this will work, but I'm planning to try it. Will let you know... :)

samdeane commented 4 years ago

Update: I think the approach I described above should work, but I've currently hit an issue.

My bridging plugin links to Sparkle, embeds the framework, and owns an SPUStandardUpdaterController object. It sets itself to be the updaterDelegate.

The various SU keys are in the main application's Info.plist, as usual.

Checking for updating appears to be working fine.

However, currently, when it finds an update, I'm getting a crash in [SPUStandardUserDriver setUpFocusForActiveUpdateAlert], calling [self.activeUpdateAlert window].

It seems that the system is actually crashing inside WebView, trying to access the alert's window:

Stack

As yet, I haven't figured out what's going on here.

samdeane commented 4 years ago

I did wonder if this was something to do with WebView being deprecated on 10.15, and the system log is showing

2020-02-26 14:29:30.695271+0000 Action Status[14298:327670] -[WebView initWithCoder:]: unrecognized selector sent to instance 0x7b180006e1c0

which looks suspiciously like WebView itself isn't there.

However, I've tried swapping out WebView for WKWebView, and I'm still getting a crash in a similar place, so I don't think it's that.

samdeane commented 4 years ago

This crash seems to be related to using WebKit from the bridging plugin, and not specifically anything to do with Sparkle. I suspect that it may be caused by WebKit getting confused about whether or not it's running under AppKit or Catalyst.

Unfortunately WebKit is used by the default driver (via SUUpdateAlert), so it may be necessary to write an alternative driver.

kornelski commented 4 years ago

Does it help if you hide release notes?

samdeane commented 4 years ago

So, I didn't try the hiding of release notes, but I suspect that it wouldn't help.

The WebView would still be in the xib, and just opening a xib-based window containing a WebView from the AppKit side of things is enough to crash.

However, I have got it working: https://twitter.com/samdeane/status/1233081407947395074

The approach I took required a bit of effort, but it allows me to present a custom Sparkle UI from the Catalyst side of things (as it happens I'm using SwiftUI for this, though it's not required).

To do this I had to create a custom Sparkle user driver (implementing the SPUUserDriver protocol), and have it forward all communication over to the Catalyst application.

The details are probably too much to go into here, but I will write it up as a blog post and drop a link here when I'm done.

samdeane commented 4 years ago

I've not yet had time to write that blog post, but I have managed to generalise the code a bit and extract it into an example which you can find here.

samdeane commented 4 years ago

FWIW I updated generalised my code a bit more and made a fairly self-contained solution.

You just link to the framework from your Catalyst app, and it deals with embedding Sparkle and loading the plugin.

tfonfara commented 3 years ago

@kornelski I've also tested a bit, unfortunately hiding the release notes doesn't help as @samdeane already expected because the nib still contains the class. But if I just remove the class from the xib everything works fine (without release notes but that's okay). Would it be possible to not instantiate the webview from the interface file but adding it from code with a "class exists" check?

tfonfara commented 3 years ago

Another idea: I just noticed this marker: WEBKIT_CLASS_DEPRECATED_MAC(10_3, 10_14, "No longer supported; please adopt WKWebView.") If WKWebView would be supported this issue would be solved automatically 😊

kornelski commented 3 years ago

WKWebView requires macOS 10.10+. I've only very recently raised minimum macOS version for Sparkle to 10.9.

tfonfara commented 3 years ago

Got it, what about a if ([WebView class]) { ... } or if (NSClassFromString(@"WebView")) { ... } check? 🤔 Like

if ([WebView class]) {
    WebView webView = [[[WebView] alloc] initWithFrame: [containerView bounds]];
    // set autoresizingMasks
    [containerView addSubview: webView];
} else {
    // set showsReleaseNotes to false to prevent crashes
}
kornelski commented 3 years ago

I know some apps use release notes to warn that an update is not free, so hiding release notes could be an unpleasant surprise.

kornelski commented 3 years ago

Maybe it would be fine to switch Sparkle 2.x only?

tfonfara commented 3 years ago

I could try, can you push the 2.x to cocoapods please (maybe as 2.x-beta)? I've just checked the repo but only the 1.x is available there 🤔

kornelski commented 3 years ago

I don't use Cocoapods. I could release 2.0-beta if you create a podspec.

tfonfara commented 3 years ago

What about the podspec that is in the project already?

zorgiepoo commented 3 years ago

@samdeane @tfonfara The story here should be somewhat better after #1704 landed (and #1709 lands for 1.x). Notably WKWebView is preferred, and the web views aren't instantiated in the nibs now. In theory WKWebView should "just work", in practice I haven't tried it out.

vincode-io commented 3 years ago

@zorgiepoo I gave it a try and there are still WKWebView problems. I guess because WKWebView is different between UIKit and AppKit? Trying to hide the release notes didn't seem to help.

2021-01-05 13:17:55.536020-0600 Zavala[45635:8477758] -[WKWebView _setSuperview:]: unrecognized selector sent to instance 0x7f8440079400
2021-01-05 13:17:55.539372-0600 Zavala[45635:8477758] [General] An uncaught exception was raised
2021-01-05 13:17:55.539455-0600 Zavala[45635:8477758] [General] -[WKWebView _setSuperview:]: unrecognized selector sent to instance 0x7f8440079400
2021-01-05 13:17:55.539572-0600 Zavala[45635:8477758] [General] (
    0   CoreFoundation                      0x00007fff204b36af __exceptionPreprocess + 242
    1   libobjc.A.dylib                     0x00007fff201eb3c9 objc_exception_throw + 48
    2   CoreFoundation                      0x00007fff20535c85 -[NSObject(NSObject) __retain_OA] + 0
    3   UIKitCore                           0x00007fff45f381e9 -[UIResponder doesNotRecognizeSelector:] + 292
    4   CoreFoundation                      0x00007fff2041b07d ___forwarding___ + 1467
    5   CoreFoundation                      0x00007fff2041aa38 _CF_forwarding_prep_0 + 120
    6   AppKit                              0x00007fff22c71cc2 -[NSView addSubview:] + 174
    7   Sparkle                             0x000000010a0e8228 -[SUUpdateAlert windowDidLoad] + 169
    8   AppKit                              0x00007fff22e29b4a -[NSWindowController _windowDidLoad] + 570
    9   AppKit                              0x00007fff22e257b2 -[NSWindowController window] + 110
    10  Sparkle                             0x000000010a0c8999 -[SPUStandardUserDriver setUpFocusForActiveUpdateAlert] + 58
    11  Sparkle                             0x000000010a0c8d5e -[SPUStandardUserDriver showUpdateFoundWithAlertHandler:] + 299
    12  Sparkle                             0x000000010a0c8e4b -[SPUStandardUserDriver showUpdateFoundWithAppcastItem:userInitiated:reply:] + 171
    13  Sparkle                             0x000000010a0cd0e3 -[SPUUIBasedUpdateDriver basicDriverDidFindUpdateWithAppcastItem:] + 375
    14  Sparkle                             0x000000010a0bd5b5 -[SPUCoreBasedUpdateDriver basicDriverDidFindUpdateWithAppcastItem:] + 356
    15  Sparkle                             0x000000010a0bc46b -[SPUBasicUpdateDriver didFindValidUpdateWithAppcastItem:] + 454
    16  Sparkle                             0x000000010a0d7d55 -[SUAppcastDriver appcastDidFinishLoading:includesSkippedUpdates:] + 976
    17  Sparkle                             0x000000010a0d7972 __96-[SUAppcastDriver loadAppcastFromURL:userAgent:httpHeaders:inBackground:includesSkippedUpdates:]_block_invoke + 110
    18  Sparkle                             0x000000010a0d59b5 __62-[SUAppcast fetchAppcastFromURL:inBackground:completionBlock:]_block_invoke + 281
    19  libdispatch.dylib                   0x0000000107ecbe78 _dispatch_call_block_and_release + 12
    20  libdispatch.dylib                   0x0000000107ecd0b0 _dispatch_client_callout + 8
    21  libdispatch.dylib                   0x0000000107edd260 _dispatch_main_queue_callback_4CF + 1107
    22  CoreFoundation                      0x00007fff20476970 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9
    23  CoreFoundation                      0x00007fff20438852 __CFRunLoopRun + 2731
    24  CoreFoundation                      0x00007fff204376ce CFRunLoopRunSpecific + 563
    25  HIToolbox                           0x00007fff28755eb0 RunCurrentEventLoopInMode + 292
    26  HIToolbox                           0x00007fff28755cac ReceiveNextEventCommon + 709
    27  HIToolbox                           0x00007fff287559cf _BlockUntilNextEventMatchingListInModeWithFilter + 64
    28  AppKit                              0x00007fff22c4fde9 _DPSNextEvent + 883
    29  AppKit                              0x00007fff22c4e5af -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1366
    30  AppKit                              0x00007fff22c40b0a -[NSApplication run] + 586
    31  AppKit                              0x00007fff22c14df2 NSApplicationMain + 816
    32  AppKit                              0x00007fff22f0b80a _NSApplicationMainWithInfoDictionary + 16
    33  UIKitMacHelper                      0x00007fff3459dffe UINSApplicationMain + 1418
    34  UIKitCore                           0x00007fff4525be90 UIApplicationMain + 144
    35  libswiftUIKit.dylib                 0x00007fff58049d42 $s5UIKit17UIApplicationMainys5Int32VAD_SpySpys4Int8VGGSgSSSgAJtF + 98
    36  Zavala                              0x00000001077ea25a $sSo21UIApplicationDelegateP5UIKitE4mainyyFZ + 122
    37  Zavala                              0x00000001077ea1ce $s6Zavala11AppDelegateC5$mainyyFZ + 46
    38  Zavala                              0x00000001077ea2a9 main + 41
    39  libdyld.dylib                       0x00007fff2035c621 start + 1
)
vincode-io commented 3 years ago

I did get this working by making one small change shown in #1713. If you make that change, disable showing release notes, and use a bundle to load Sparkle, it works. This wouldn't have been possible without #1704, so many thanks for that change.

Showing release notes in a Catalyst app with Sparkle is something I suspect isn't possible. The UIKit WKWebView vs. the AppKit WKWebView are probably too different.

At least this gets those of us doing Catalyst apps a way to distribute test builds, since there still isn't TestFlight for the Mac yet.

tfonfara commented 3 years ago

Actually I figured out that even without any Sparkle code at all, if run this:

let view = NSView(frame: NSRect(x: 0, y: 0, width: 100, height: 100))
view.addSubview(WKWebView(frame: NSRect(x: 0, y: 0, width: 100, height: 100), configuration: WKWebViewConfiguration()))

within my AppKit bundle, it crashes with same error.

I've created a related bug report at feedbackassistant.apple.com.

zorgiepoo commented 3 years ago

Yeah I guess WKWebView isn't a NSView in this case which sounds quite odd to me.. but I'm not so familiar with Catalyst.

Thanks for #1713 which is a good addition and for filing the Feedback report.

And kudos for trying the custom user driver / UI approach here.

tfonfara commented 3 years ago

I've adopted the changes of @vincode-io for the 1.x version in #1715

tfonfara commented 3 years ago

FYI, I just checked the status of my feedback report, it is set to: Resolution: Investigation complete - Works as currently designed...

zorgiepoo commented 3 years ago

It then seems like if you want to use a web view to show release notes in a Catalyst app, you need to show the web view on the iOS side of your app, which would entail a custom user driver (2.x). Hiding release notes is probably simpler for majority.

zorgiepoo commented 3 years ago

Catalyst is more documented on https://sparkle-project.org/documentation/programmatic-setup/ now.

WeirdHat commented 2 years ago

Hello - I have run into this problem, and hiding release notes doesn't seem like a satisfactory solution, nor does reinventing the wheel by replacing the whole window with a custom UI.

How feasible would it be to display release notes with something other than a WKWebView? I believe NSAttributedString can be created from HTML, and displayed in an NSTextView?

Failing that, can anybody who's already solved this with a custom UI share their code?

zorgiepoo commented 2 years ago

sparkle-cli has a custom UI (or lack of one) that creates an NSAttributedString from the HTML and prints it to the console. So if you create a custom UI, it should be possible.

zorgiepoo commented 2 years ago

If you want Sparkle to automatically use a NSTextView for Catalyst apps then maybe that is possible as well (maybe would involve a new SUWebView implementation). I don't know if it will look good enough to "ship" though and someone would have to contribute to it.

WeirdHat commented 2 years ago

Okay, so I think I got it working well enough for my own purposes, with an SUWebView implementation that does this:

- (instancetype)initWithColorStyleSheetLocation:(NSURL *)colorStyleSheetLocation fontFamily:(NSString *)fontFamily fontPointSize:(int)fontPointSize javaScriptEnabled:(BOOL)javaScriptEnabled
{
    self = [super init];
    if (self != nil) {
        _scrollView = [[NSScrollView alloc] initWithFrame: NSZeroRect];
        _textView = [[NSTextView alloc] initWithFrame: NSZeroRect];
        _scrollView.documentView = _textView;
    }
    return self;
}

- (NSView *)view
{
    return self.scrollView;
}

- (void)loadHTMLString:(NSString *)htmlString baseURL:(NSURL * _Nullable)baseURL completionHandler:(void (^)(NSError * _Nullable))completionHandler
{
    self.completionHandler = [completionHandler copy];

    NSData* data = [htmlString dataUsingEncoding:NSUTF8StringEncoding];
    NSAttributedString *attributedString = [[NSAttributedString alloc] initWithHTML:data documentAttributes:nil];
    [[self.textView textStorage] setAttributedString:attributedString];

    NSSize contentSize = [_scrollView contentSize];
    [_textView setFrame:NSMakeRect(0, 0, contentSize.width, contentSize.height)];
    [_textView setMinSize:NSMakeSize(0.0, contentSize.height)];
    [_textView setMaxSize:NSMakeSize(FLT_MAX, FLT_MAX)];
    [_textView setVerticallyResizable:YES];
    [_textView setHorizontallyResizable:NO];
    [_textView setAutoresizingMask:NSViewWidthSizable];
    [_textView setTextContainerInset:NSMakeSize(8, 8)];
    _textView.editable = NO;

    [_scrollView setHasVerticalScroller:YES];
    [_scrollView setHasHorizontalScroller:NO];

    if (self.completionHandler != nil) {
        self.completionHandler(nil);
        self.completionHandler = nil;
    }
}

Like you said, not good enough to ship, but hopefully this is a helpful start if anybody wants/needs to take it further.

zorgiepoo commented 2 years ago

Nice. How does it look though or what’s lacking?

WeirdHat commented 2 years ago

Looks basically fine, except it's not changing the text color for dark mode (that might be an easy fix though)

Screen Shot 2022-09-08 at 9 35 46 PM

But some things it lacks that I don't need personally:

zorgiepoo commented 2 years ago

Doesn't support the styling that the webview does.

The styling from the attributed string will be as much as we can get here.

Doesn't support javascript.

Yeah this is expected (Sparkle also disables javascript by default generally; most apps don't use it).

Doesn't check whether it's running in a Catalyst app or not - I'm not sure it's actually possible to know that, since it has to be run from a plugin that's separate from the Catalyst part of the app, so it might have to be a manual option.

I think you can use -[NSProcessInfo macCatalystApp]

Link clicking and dark mode should be possible or at least TextEdit can do it. I think this would be nice over no release notes.

zorgiepoo commented 2 years ago

Dark mode should be usesAdaptiveColorMappingForDarkAppearance property.

Link clicking might need to override this delegate method https://developer.apple.com/documentation/appkit/nstextviewdelegate/1449527-textview?language=objc - for web views we do have some "safe link" handling and only allow certain url schemes.

zorgiepoo commented 2 years ago

I looked into this more and tried to do a more complete implementation based on @WeirdHat 's snippet however it cannot be shipped.

The NSAttributedString HTML reader APIs use WebKit underneath. If you pass any HTML options like I tried to do, a Catalyst app will crash with the stacktrace pointing to WebKit usage. If you don't pass any options, it will "work" (for now) but I consider this brittle and easily breakable in the future.

To use NSAttributedString+HTML, I think there needs to be an out-of-process (XPC) implementation. If you want to support a non-HTML path, this could technically be safe and do-able today though (but I consider this somewhat incomplete).

zorgiepoo commented 1 year ago

I added support for a plain text view for release notes in #2315

After thinking about it, I don't like the idea of converting HTML into a NSAttributedString. I don't want it to (potentially) go through WebKit and it also didn't always give the result I'd expect if I recall correctly.

I think markdown support in the future would make a better alternative #2319