Bug 58136 - View.SystemUiVisibility has wrong type
Summary: View.SystemUiVisibility has wrong type
Status: CONFIRMED
Alias: None
Product: Android
Classification: Xamarin
Component: Bindings ()
Version: unspecified
Hardware: PC Mac OS
: Highest major
Target Milestone: abi-break-future
Assignee: Atsushi Eno
URL:
Depends on:
Blocks:
 
Reported: 2017-07-13 12:02 UTC by Marco Burato
Modified: 2017-07-24 08:11 UTC (History)
4 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report for Bug 58136 on Developer Community or GitHub if you have new information to add and do not yet see a matching new report.

If the latest results still closely match this report, you can use the original description:

  • Export the original title and description: Developer Community HTML or GitHub Markdown
  • Copy the title and description into the new report. Adjust them to be up-to-date if needed.
  • Add your new information.

In special cases on GitHub you might also want the comments: GitHub Markdown with public comments

Related Links:
Status:
CONFIRMED

Description Marco Burato 2017-07-13 12:02:41 UTC
The property View.SystemUiVisibility should have type SystemUiFlags but it's StatusBarVisibility instead.
Comment 1 Jon Douglas [MSFT] 2017-07-13 16:43:42 UTC
Based on the Android docs found here:

https://developer.android.com/reference/android/view/View.html#setSystemUiVisibility(int)

The respective parameters can be one of the following:

int: Bitwise-or of flags 

SYSTEM_UI_FLAG_LOW_PROFILE, SYSTEM_UI_FLAG_HIDE_NAVIGATION, SYSTEM_UI_FLAG_FULLSCREEN, SYSTEM_UI_FLAG_LAYOUT_STABLE, SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION, SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN, SYSTEM_UI_FLAG_IMMERSIVE, and SYSTEM_UI_FLAG_IMMERSIVE_STICKY.

Thus StatusBarVisibility is definitely not the correct enum as it only presents two options: Hidden, Visible.

Thus I believe that the two are incorrect:

https://github.com/xamarin/xamarin-android/blob/1889b58257130b1db5e01104ab59183e5bdf67a7/src/Mono.Android/methodmap.csv#L318-L319

and rather they should be of type:

Android.Views.SystemUiFlags

https://developer.xamarin.com/api/type/Android.Views.SystemUiFlags/

Thus I am setting this bug to CONFIRMED and setting the priority to HIGHEST MAJOR due to an API issue similar to the following bug:

https://bugzilla.xamarin.com/show_bug.cgi?id=51664
Comment 2 Jonathan Pryor 2017-07-14 20:44:42 UTC
This can't be fixed. Changing the type of `View.SystemUiVisibility` would be an ABI break, and we don't want to incur non-necessary ABI breaks.

We *could* plausibly "overload" this property, e.g. a `SystemUiVisibilityEx` property (bleh?), but it *couldn't* be virtual. (It's not a good idea to have 2+ C# virtual members which bind the same Java API, as then you have a problem: which one is actually called when the Java method is called from Java-land?)
Comment 3 Marco Burato 2017-07-17 08:50:58 UTC
Mmm... aren't all enums just ints under the hood? That wouldn't be an ABI break.
Right now, to workaround the issue, I'm casting the correct SystemUiFlags to StatusBarVisibility to make it compile and it works.

Also, isn't Xamarin.Android bundled with the app anyway? I can't picture a scenario where an old app assembly gets run with a newer Xamarin assembly.
Looks like the worst case would be for an existing piece of code to stop compiling after a Xamarin.Android upgrade. Code which probably doesn't exist because that property is basically unusable as it is.
Comment 4 Jonathan Pryor 2017-07-21 16:09:12 UTC
@Marco asked:

> Mmm... aren't all enums just ints under the hood? That wouldn't be an ABI break.

That *would* be an ABI break.

Apparently we don't have a clear understanding on "ABI break." Thus, an example.

Suppose we have our "library" assembly:

  // lib.cs;
  // Compile with: csc /t:library lib.cs
  namespace Example {
    using System;
    public class View {
      public StringComparison Comparison { get; }
    }
  }

Our app assembly:

  // app.cs
  // Compile with: csc app.cs /r:lib.dll
  using Example;
  using System;
  class App {
    public static void Main ()
    {
      var v = new View ();
      Console.WriteLine (v.Comparison);
    }
  }

The app assembly executes as expected:

  $ mono app.exe
  CurrentCulture

If we *change* our library assembly so that View.Comparison is instead a TypeCode:

    public class View {
      public TypeCode Comparison { get; }
    }

Then re-run app.exe *without changing it*, everything breaks:

  $ csc /t:library lib.cs
  $ mono app.exe
  Unhandled Exception:
> System.MissingMethodException: Method 'Example.View.get_Comparison' not found.
> [ERROR] FATAL UNHANDLED EXCEPTION: System.MissingMethodException: Method 'Example.View.get_Comparison' not found.

THIS is what I mean by "ABI break." The fact that the type is an enum is *irrelevant*. The fact that values of the type can be cast to other types is irrelevant. What matters are the actual types, as contained in the assembly: System.StringComparison != System.TypeCode. (Note in particular that both StringComparison and TypeCode are enumeration types with `int` as the underlying enum type.)

We CANNOT change the actual compile-time type of View.SystemUiFlags without breaking all existing references to that member.

> isn't Xamarin.Android bundled with the app anyway?

This is correct, and also not relevant.

> I can't picture a scenario where an old app assembly gets
> run with a newer Xamarin assembly.

The problem isn't "apps." The problem is *assemblies*. Assemblies such as those in the Xamarin Component store, or on NuGet.org, or emailed to you from your friend. ANY assembly that uses `View.SystemUiFlags` will BREAK if the type changes.

This is an unknowable set of assemblies which could potentially break.
Comment 5 Marco Burato 2017-07-24 08:11:57 UTC
Thank you for the explanation Jonathan.
I was thinking in terms of unmanaged code, say a C function, but now I understand that changing the .NET method signature would indeed be an ABI break.
Then, I didn't consider other assemblies possibly using the property, which makes the ABI break a real problem if someone has already worked-around the issue casting enums.