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

Fix Developer Tools option setting for Blazor Hybrid Mac Catalyst view. #19909

Merged

Conversation

drasticactions
Copy link
Contributor

Description of Change

I had accidently set the version of the OperatingSystem.IsMacCatalystVersionAtLeast check to match the literal version of macOS, when it needed to be the version of iOS that that specific Mac Catalyst version was targeting, which should be 16.6. Changing it should make it so it no longer throws for the platform. It was not caught as a regression as all of the tests for Blazor run on Macs new enough to support that flag.

Also, since we need to use selectors to trigger that setting, and to be totally sure there can't be a crash should that number still be incorrect, it will only be enabled now if DeveloperTools.Enabled is set, instead of calling that method and setting the bool all the time. Since it's false by default, this should be fine.

Issues Fixed

Fixes #19613

@drasticactions drasticactions requested a review from a team as a code owner January 15, 2024 16:01
@jsuarezruiz jsuarezruiz added the area/tooling ⚙️ XAML & C# Hot Reload, XAML Editor, Live Visual Tree, Live Preview, Debugging label Jan 16, 2024
@jfversluis jfversluis enabled auto-merge (squash) January 16, 2024 19:43
@Eilon
Copy link
Member

Eilon commented Jan 19, 2024

And does this need to be back-ported to net8 as well?

@drasticactions
Copy link
Contributor Author

drasticactions commented Jan 19, 2024

And does this need to be back-ported to net8 as well?

Yeah. I'm not sure if that's automatically happening still (#19898 was a few days ago) but if not we can do that.

Also for testing, I did try this by building and deploying a Blazor Hybrid app (Via the template) on macOS 12 via a VM, with the setting on and off, and it worked correctly. The setting itself was only invoked for the "classic" path and avoided the new selector. I couldn't get a macOS 11.x machine so if anyone had access to one to try it, that would be great, but it should be fine there as well. The worse case would be that if you were actively developing on it (which I don't think is possible because of Xcode not being supported on it) is you would turn off developer mode for the web view.

@jfversluis jfversluis merged commit 5305f1a into dotnet:main Jan 19, 2024
47 checks passed
@MeestorX
Copy link

I'd be happy to test this on MacOS 11, as I also reported this issue and would like to know if the fix works there. Are there instructions on how to download a build of .NET SDK with this fix applied? (Sorry for the newbie question)

@Eilon Eilon added area/blazor 🕸️ Blazor Hybrid / Desktop, BlazorWebView and removed area/tooling ⚙️ XAML & C# Hot Reload, XAML Editor, Live Visual Tree, Live Preview, Debugging labels Feb 8, 2024
@Eilon
Copy link
Member

Eilon commented Feb 8, 2024

@drasticactions - can we ensure this is ported to .NET 8 as well? Looks like some folks are hitting it there already. I filed #20451 to track.

@drasticactions
Copy link
Contributor Author

@Eilon It should already be: https://github.com/dotnet/maui/commits/net8.0/src/BlazorWebView/src/Maui/iOS/BlazorWebViewHandler.iOS.cs

I think it was held up by the syncing of main to net8.0 not happening until recently, but it would probably go out in the next SR?

@drasticactions drasticactions deleted the dev/timill/blazor-version-check branch February 9, 2024 04:22
@Eilon
Copy link
Member

Eilon commented Feb 9, 2024

@drasticactions great, then I closed the linked bug I created.

@MeestorX
Copy link

MeestorX commented Feb 9, 2024

Does this actually fix my issue?

@Eilon
Copy link
Member

Eilon commented Feb 9, 2024

@MeestorX the patch should fix your issue once it's out. But I also posted a possible workaround here: #19613 (comment)

@drasticactions
Copy link
Contributor Author

drasticactions commented Feb 9, 2024

I think it’s also in the nightly builds, so if you upgrade to that it should work. If you upgraded and it doesn’t work, try turning off the developer tools option for Blazor Web View. If it still doesn’t work, then it could be something else.

@MeestorX
Copy link

MeestorX commented Feb 9, 2024

Can't try nightly builds due to this issue: dotnet/sdk#38636
FYI, I'm testing on release builds that don't use developer tools. The workaround didn't fix the issue.

@drasticactions
Copy link
Contributor Author

@MeestorX the original bug was that I was calling an Objective C selector to enable or disable the dev tools support, and it was being accidentally called on OS versions that didn’t support it. Even if you turned it off, it would still call it, hence it crashing in all builds.

the fix was to make sure that only OS versions that supported it would call that function, and if you turned it off that it wouldn’t attempt to call it in the first place.

And FWIW you could probably not try changing MauiVersion and change the Nuget version directly on the packages. Also, upgrading your workload versions to ones above .NET 8.0 RC1 should help.

@MeestorX
Copy link

Ok, sorry, but I'm not sure where to go from here. I'd love to test out anything that fixes this as it's a real showstopper for me.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/blazor 🕸️ Blazor Hybrid / Desktop, BlazorWebView
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Blazor Hybrid is not supported on Macs running MacOS 11.
5 participants