From ce16ebea45b3a0367f5dc46d13f2ddbcb54cdeb6 Mon Sep 17 00:00:00 2001 From: "E.Z. Hart" Date: Thu, 22 Jun 2017 16:33:03 -0600 Subject: [PATCH] Set the Id field for Android Views created by Forms #1004 --- .../MapsModalCrash.cs | 86 ++++++++++++++++++++++ .../Xamarin.Forms.Controls.Issues.Shared.projitems | 1 + Xamarin.Forms.Controls/TestCases.cs | 2 +- .../AppCompat/CarouselPageRenderer.cs | 2 +- .../AppCompat/FormsAppCompatActivity.cs | 17 ----- .../AppCompat/MasterDetailContainer.cs | 2 +- .../AppCompat/NavigationPageRenderer.cs | 2 +- .../AppCompat/Platform.cs | 7 ++ .../AppCompat/TabbedPageRenderer.cs | 2 +- .../FastRenderers/AutomationPropertiesProvider.cs | 4 +- .../FastRenderers/ButtonRenderer.cs | 19 +++-- .../FastRenderers/FrameRenderer.cs | 2 + .../FastRenderers/ImageRenderer.cs | 5 +- .../FastRenderers/LabelRenderer.cs | 2 + Xamarin.Forms.Platform.Android/Platform.cs | 17 +++++ .../Renderers/PageContainer.cs | 1 + .../Renderers/PageRenderer.cs | 5 ++ Xamarin.Forms.Platform.Android/ViewExtensions.cs | 13 ++++ Xamarin.Forms.Platform.Android/ViewRenderer.cs | 8 +- 19 files changed, 161 insertions(+), 36 deletions(-) create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/MapsModalCrash.cs diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/MapsModalCrash.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/MapsModalCrash.cs new file mode 100644 index 0000000..fcae484 --- /dev/null +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/MapsModalCrash.cs @@ -0,0 +1,86 @@ +using System; +using Xamarin.Forms.CustomAttributes; +using Xamarin.Forms.Internals; +using Xamarin.Forms.Maps; +#if UITEST +using Xamarin.Forms.Core.UITests; +using Xamarin.UITest; +using NUnit.Framework; +#endif + +namespace Xamarin.Forms.Controls.Issues +{ + +#if UITEST + [Category(UITestCategories.Maps)] + [Category(UITestCategories.ManualReview)] +#endif + + [Preserve(AllMembers = true)] + [Issue(IssueTracker.None, 1701, "Modal Page over Map crashes application", PlatformAffected.Android)] + public class MapsModalCrash : TestContentPage + { + const string StartTest = "Start Test"; + const string DisplayModal = "Click Me"; + const string Success = "SuccessLabel"; + + protected override void Init() + { + var button = new Button { Text = StartTest }; + button.Clicked += (sender, args) => + { + Application.Current.MainPage = MapPage(); + }; + + Content = new StackLayout + { + Padding = new Thickness(0, 20, 0, 0), + Children = + { + button + } + }; + } + + static ContentPage MapPage() + { + var map = new Map(); + + var button = new Button { Text = DisplayModal }; + button.Clicked += (sender, args) => button.Navigation.PushModalAsync(new NavigationPage(SuccessPage())); + + return new ContentPage + { + Content = new StackLayout + { + Children = + { + map, + button + } + } + }; + } + + static ContentPage SuccessPage() + { + return new ContentPage + { + BackgroundColor = Color.LightBlue, + Content = new Label { Text = "If you're seeing this, then the test was a success.", AutomationId = Success } + }; + } + +#if UITEST + [Test] + public void CanDisplayModalOverMap() + { + RunningApp.WaitForElement(StartTest); + RunningApp.Tap(StartTest); + RunningApp.WaitForElement(DisplayModal); + RunningApp.Tap(DisplayModal); + RunningApp.WaitForElement(Success); + } +#endif + } +} \ No newline at end of file diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems index 746205d..eb75116 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems @@ -235,6 +235,7 @@ + diff --git a/Xamarin.Forms.Controls/TestCases.cs b/Xamarin.Forms.Controls/TestCases.cs index d559478..cd49b08 100644 --- a/Xamarin.Forms.Controls/TestCases.cs +++ b/Xamarin.Forms.Controls/TestCases.cs @@ -141,7 +141,7 @@ namespace Xamarin.Forms.Controls var untrackedIssueCells = from issueModel in issueModels where issueModel.IssueTracker == IssueTracker.None - orderby issueModel.Description + orderby issueModel.IssueNumber descending, issueModel.Description select MakeIssueCell (issueModel.Name, issueModel.Description, issueModel.Action); var issueCells = bugzillaIssueCells.Concat (githubIssueCells).Concat (untrackedIssueCells); diff --git a/Xamarin.Forms.Platform.Android/AppCompat/CarouselPageRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/CarouselPageRenderer.cs index 766ee22..f50815d 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/CarouselPageRenderer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/CarouselPageRenderer.cs @@ -94,7 +94,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat LayoutParameters = new ViewGroup.LayoutParams(ViewGroup.LayoutParams.MatchParent, ViewGroup.LayoutParams.MatchParent), Adapter = new FormsFragmentPagerAdapter(e.NewElement, activity.SupportFragmentManager) { CountOverride = e.NewElement.Children.Count } }; - pager.Id = FormsAppCompatActivity.GetUniqueId(); + pager.Id = Platform.GenerateViewId(); pager.AddOnPageChangeListener(this); ViewGroup.AddView(pager); diff --git a/Xamarin.Forms.Platform.Android/AppCompat/FormsAppCompatActivity.cs b/Xamarin.Forms.Platform.Android/AppCompat/FormsAppCompatActivity.cs index 8bba063..a121cff 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/FormsAppCompatActivity.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/FormsAppCompatActivity.cs @@ -549,23 +549,6 @@ namespace Xamarin.Forms.Platform.Android public static int ToolbarResource { get; set; } - internal static int GetUniqueId() - { - // getting unique Id's is an art, and I consider myself the Jackson Pollock of the field - if ((int)Build.VERSION.SdkInt >= 17) - return global::Android.Views.View.GenerateViewId(); - - // Numbers higher than this range reserved for xml - // If we roll over, it can be exceptionally problematic for the user if they are still retaining things, android's internal implementation is - // basically identical to this except they do a lot of locking we don't have to because we know we only do this - // from the UI thread - if (s_id >= 0x00ffffff) - s_id = 0x00000400; - return s_id++; - } - - static int s_id = 0x00000400; - #endregion } } diff --git a/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs b/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs index 5084092..faaf746 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs @@ -19,7 +19,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat public MasterDetailContainer(MasterDetailPage parent, bool isMaster, Context context) : base(parent, isMaster, context) { - Id = FormsAppCompatActivity.GetUniqueId(); + Id = Platform.GenerateViewId(); _parent = parent; _isMaster = isMaster; } diff --git a/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs index d405b42..69e1c13 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs @@ -50,7 +50,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat public NavigationPageRenderer() { AutoPackage = false; - Id = FormsAppCompatActivity.GetUniqueId(); + Id = Platform.GenerateViewId(); Device.Info.PropertyChanged += DeviceInfoPropertyChanged; } diff --git a/Xamarin.Forms.Platform.Android/AppCompat/Platform.cs b/Xamarin.Forms.Platform.Android/AppCompat/Platform.cs index 872bac7..0081d5a 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/Platform.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/Platform.cs @@ -353,6 +353,8 @@ namespace Xamarin.Forms.Platform.Android.AppCompat Android.Platform.SetRenderer(modal, _renderer); AddView(_renderer.View); + + Id = Platform.GenerateViewId(); } protected override void Dispose(bool disposing) @@ -399,6 +401,11 @@ namespace Xamarin.Forms.Platform.Android.AppCompat } } + internal static int GenerateViewId() + { + return Android.Platform.GenerateViewId(); + } + #region Statics public static implicit operator ViewGroup(Platform canvas) diff --git a/Xamarin.Forms.Platform.Android/AppCompat/TabbedPageRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/TabbedPageRenderer.cs index bb14100..78fc19c 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/TabbedPageRenderer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/TabbedPageRenderer.cs @@ -166,7 +166,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat LayoutParameters = new LayoutParams(LayoutParams.MatchParent, LayoutParams.MatchParent), Adapter = new FormsFragmentPagerAdapter(e.NewElement, FragmentManager) { CountOverride = e.NewElement.Children.Count } }; - pager.Id = FormsAppCompatActivity.GetUniqueId(); + pager.Id = Platform.GenerateViewId(); pager.AddOnPageChangeListener(this); AddView(pager); diff --git a/Xamarin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs b/Xamarin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs index 6cd158f..e4118ac 100644 --- a/Xamarin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs +++ b/Xamarin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs @@ -162,8 +162,8 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers if (elemValue != null) { var id = Control.Id; - if (id == -1) - id = Control.Id = FormsAppCompatActivity.GetUniqueId(); + if (id == global::Android.Views.View.NoId) + id = Control.Id = Platform.GenerateViewId(); var renderer = elemValue?.GetRenderer(); renderer?.SetLabelFor(id); diff --git a/Xamarin.Forms.Platform.Android/FastRenderers/ButtonRenderer.cs b/Xamarin.Forms.Platform.Android/FastRenderers/ButtonRenderer.cs index 6d19ffe..3c08f4b 100644 --- a/Xamarin.Forms.Platform.Android/FastRenderers/ButtonRenderer.cs +++ b/Xamarin.Forms.Platform.Android/FastRenderers/ButtonRenderer.cs @@ -40,7 +40,7 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers _effectControlProvider = new EffectControlProvider(this); _textColorSwitcher = new Lazy(() => new TextColorSwitcher(TextColors)); - Initialize(); + Initialize(); } public VisualElement Element => Button; @@ -208,13 +208,16 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers { if (e.NewElement != null && !_isDisposed) { - UpdateFont(); - UpdateText(); - UpdateBitmap(); - UpdateTextColor(); - UpdateIsEnabled(); - UpdateInputTransparent(); - UpdateBackgroundColor(); + + this.EnsureId(); + + UpdateFont(); + UpdateText(); + UpdateBitmap(); + UpdateTextColor(); + UpdateIsEnabled(); + UpdateInputTransparent(); + UpdateBackgroundColor(); } ElementChanged?.Invoke(this, new VisualElementChangedEventArgs(e.OldElement, e.NewElement)); diff --git a/Xamarin.Forms.Platform.Android/FastRenderers/FrameRenderer.cs b/Xamarin.Forms.Platform.Android/FastRenderers/FrameRenderer.cs index d3583cc..82637a3 100644 --- a/Xamarin.Forms.Platform.Android/FastRenderers/FrameRenderer.cs +++ b/Xamarin.Forms.Platform.Android/FastRenderers/FrameRenderer.cs @@ -145,6 +145,8 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers if (e.NewElement != null) { + this.EnsureId(); + if (_visualElementTracker == null) { _visualElementTracker = new VisualElementTracker(this); diff --git a/Xamarin.Forms.Platform.Android/FastRenderers/ImageRenderer.cs b/Xamarin.Forms.Platform.Android/FastRenderers/ImageRenderer.cs index 7690102..28aa11a 100644 --- a/Xamarin.Forms.Platform.Android/FastRenderers/ImageRenderer.cs +++ b/Xamarin.Forms.Platform.Android/FastRenderers/ImageRenderer.cs @@ -62,11 +62,12 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers { await TryUpdateBitmap(e.OldElement); UpdateAspect(); + this.EnsureId(); ElementChanged?.Invoke(this, new VisualElementChangedEventArgs(e.OldElement, e.NewElement)); } - public override bool OnTouchEvent(MotionEvent e) + public override bool OnTouchEvent(MotionEvent e) { bool handled; var result = _visualElementRenderer.OnTouchEvent(e, Parent, out handled); @@ -74,7 +75,7 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers return handled ? result : base.OnTouchEvent(e); } - protected virtual Size MinimumSize() + protected virtual Size MinimumSize() { return new Size(); } diff --git a/Xamarin.Forms.Platform.Android/FastRenderers/LabelRenderer.cs b/Xamarin.Forms.Platform.Android/FastRenderers/LabelRenderer.cs index aa1f06e..f4b11bb 100644 --- a/Xamarin.Forms.Platform.Android/FastRenderers/LabelRenderer.cs +++ b/Xamarin.Forms.Platform.Android/FastRenderers/LabelRenderer.cs @@ -177,6 +177,8 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers if (e.NewElement != null) { + this.EnsureId(); + if (_visualElementTracker == null) { _visualElementTracker = new VisualElementTracker(this); diff --git a/Xamarin.Forms.Platform.Android/Platform.cs b/Xamarin.Forms.Platform.Android/Platform.cs index fe588eb..bbbdf95 100644 --- a/Xamarin.Forms.Platform.Android/Platform.cs +++ b/Xamarin.Forms.Platform.Android/Platform.cs @@ -1035,6 +1035,23 @@ namespace Xamarin.Forms.Platform.Android } } + internal static int GenerateViewId() + { + // getting unique Id's is an art, and I consider myself the Jackson Pollock of the field + if ((int)Build.VERSION.SdkInt >= 17) + return global::Android.Views.View.GenerateViewId(); + + // Numbers higher than this range reserved for xml + // If we roll over, it can be exceptionally problematic for the user if they are still retaining things, android's internal implementation is + // basically identical to this except they do a lot of locking we don't have to because we know we only do this + // from the UI thread + if (s_id >= 0x00ffffff) + s_id = 0x00000400; + return s_id++; + } + + static int s_id = 0x00000400; + internal class DefaultRenderer : VisualElementRenderer { bool _notReallyHandled; diff --git a/Xamarin.Forms.Platform.Android/Renderers/PageContainer.cs b/Xamarin.Forms.Platform.Android/Renderers/PageContainer.cs index a4e7736..435ee7c 100644 --- a/Xamarin.Forms.Platform.Android/Renderers/PageContainer.cs +++ b/Xamarin.Forms.Platform.Android/Renderers/PageContainer.cs @@ -10,6 +10,7 @@ namespace Xamarin.Forms.Platform.Android AddView(child.View); Child = child; IsInFragment = inFragment; + Id = Platform.GenerateViewId(); } public IVisualElementRenderer Child { get; set; } diff --git a/Xamarin.Forms.Platform.Android/Renderers/PageRenderer.cs b/Xamarin.Forms.Platform.Android/Renderers/PageRenderer.cs index 7db4a4a..3474abc 100644 --- a/Xamarin.Forms.Platform.Android/Renderers/PageRenderer.cs +++ b/Xamarin.Forms.Platform.Android/Renderers/PageRenderer.cs @@ -43,6 +43,11 @@ namespace Xamarin.Forms.Platform.Android Page view = e.NewElement; base.OnElementChanged(e); + if (Id == NoId) + { + Id = Platform.GenerateViewId(); + } + UpdateBackgroundColor(view); UpdateBackgroundImage(view); diff --git a/Xamarin.Forms.Platform.Android/ViewExtensions.cs b/Xamarin.Forms.Platform.Android/ViewExtensions.cs index 6afcf5c..8914670 100644 --- a/Xamarin.Forms.Platform.Android/ViewExtensions.cs +++ b/Xamarin.Forms.Platform.Android/ViewExtensions.cs @@ -64,5 +64,18 @@ namespace Xamarin.Forms.Platform.Android } } } + + public static void EnsureId(this AView view) + { + if (view.IsDisposed()) + { + return; + } + + if (view.Id == AView.NoId) + { + view.Id = Platform.GenerateViewId(); + } + } } } \ No newline at end of file diff --git a/Xamarin.Forms.Platform.Android/ViewRenderer.cs b/Xamarin.Forms.Platform.Android/ViewRenderer.cs index 1cf7759..dbd930b 100644 --- a/Xamarin.Forms.Platform.Android/ViewRenderer.cs +++ b/Xamarin.Forms.Platform.Android/ViewRenderer.cs @@ -290,6 +290,10 @@ namespace Xamarin.Forms.Platform.Android _container = container; Control = control; + if (Control.Id == NoId) + { + Control.Id = Platform.GenerateViewId(); + } AView toAdd = container == this ? control : (AView)container; AddView(toAdd, LayoutParams.MatchParent); @@ -310,8 +314,8 @@ namespace Xamarin.Forms.Platform.Android if (elemValue != null) { var id = Control.Id; - if (id == -1) - id = Control.Id = FormsAppCompatActivity.GetUniqueId(); + if (id == NoId) + id = Control.Id = Platform.GenerateViewId(); var renderer = elemValue?.GetRenderer(); renderer?.SetLabelFor(id); -- 2.7.4