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

[Mono.Android] Fix incorrect Context.RegisterReceiver enumification. #7735

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Jan 24, 2023

Fixes #7503

Android.Content.Context.RegisterReceiver(BroadcastReceiver, IntentFilter, int) (+1 overload) was incorrectly enumified as ActivityFlags when it should have been ReceiverFlags.

To fix without breaking API:

  • Create new overloads methods that chain to the existing incorrectly enumified methods.
  • Mark the existing methods as [Obsolete] with description.

Additionally, it was noticed that the public API tracking added in #8171 did not get flagged as an error, only a warning. Change to an error to ensure any new public API is accounted for.

@jpobst jpobst force-pushed the register-receiver branch 2 times, most recently from fc5a525 to a7a8fb1 Compare February 6, 2023 18:30
@jpobst jpobst force-pushed the register-receiver branch 2 times, most recently from 4637626 to 2f4b164 Compare July 25, 2023 15:21
@jpobst jpobst marked this pull request as ready for review July 25, 2023 19:07
@jpobst jpobst requested a review from jonpryor as a code owner July 25, 2023 19:07
@jonpryor
Copy link
Member

What gives me pause is that the resulting binding makes an abstract method [Obsolete], a'la:

partial class Context {
    [Obsolete ("This method has an incorrect enumeration type. Use the overload that takes ReceiverFlags instead.")]
    public abstract Android.Content.Intent? RegisterReceiver (Android.Content.BroadcastReceiver? receiver, Android.Content.IntentFilter? filter, [global::Android.Runtime.GeneratedEnum] Android.Content.ActivityFlags flags);
}

which I believe means that every override also needs to be [Obsolete], hence the changes in this PR to ContextWrapper and MockContext. This feels "weird".

On the plus side, (effectively) nobody should ever override Context.RegisterReceiver(), so this should be safe.

@jpobst
Copy link
Contributor Author

jpobst commented Jul 25, 2023

This feels "weird".

Agreed.

On the plus side, (effectively) nobody should ever override Context.RegisterReceiver(), so this should be safe.

This is the rationale I eventually went with. ~No one should override this, users should only be calling the system provided version(s) of it.

@jonpryor jonpryor merged commit d9e4407 into main Jul 25, 2023
49 checks passed
@jonpryor jonpryor deleted the register-receiver branch July 25, 2023 20:26
grendello added a commit to grendello/xamarin-android that referenced this pull request Jul 25, 2023
* main:
  Bump to dotnet/installer@f8bab721ae 8.0.100-rc.1.23373.1 (dotnet#8202)
  [Mono.Android] Fix Context.RegisterReceiver() enumification (dotnet#7735)
grendello added a commit to grendello/xamarin-android that referenced this pull request Jul 27, 2023
* main:
  [ci] Remove .NET branches from classic release trigger (dotnet#8218)
  Bump to dotnet/installer@f8bab721ae 8.0.100-rc.1.23373.1 (dotnet#8202)
  [Mono.Android] Fix Context.RegisterReceiver() enumification (dotnet#7735)
  [ci] Add MAUI integration job (dotnet#8200)
  [vs-workload] Set EnableSideBySideManifests=true (dotnet#8179)
grendello added a commit to grendello/xamarin-android that referenced this pull request Sep 4, 2023
* main:
  Bump to dotnet/installer@f8bab721ae 8.0.100-rc.1.23373.1 (dotnet#8202)
  [Mono.Android] Fix Context.RegisterReceiver() enumification (dotnet#7735)
  [ci] Add MAUI integration job (dotnet#8200)
@github-actions github-actions bot locked and limited conversation to collaborators Jan 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Argument type of Context.RegisterReceiver() is different from native API
2 participants