Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Fix Entry crashing on iOS < 14 #14526

Merged
merged 2 commits into from
Oct 8, 2021
Merged

Conversation

rotorgames
Copy link
Contributor

@rotorgames rotorgames commented Aug 23, 2021

Description of Change

Fixing crashes on iOS < 14 if ClearButtonVisibility of Entry is WhileEditing

Issues Resolved

Fixes:

API Changes

None

Platforms Affected

  • iOS

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

  • Use iOS simulator with iOS 13 or lower
  • Set ClearButtonVisibility of Entry to WhileEditing
  • The app must not crash when a page is opening

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@jfversluis jfversluis added p/iOS 🍎 t/bug 🐛 i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often labels Sep 6, 2021
# Conflicts:
#	Xamarin.Forms.Platform.iOS/Renderers/EntryRenderer.cs
@rotorgames
Copy link
Contributor Author

@jfversluis Re-open this, please. Explanations: #14566 (comment)

@jfversluis
Copy link
Member

Reopening this to make sure we have another look

@jfversluis jfversluis reopened this Sep 21, 2021
@jfversluis jfversluis added this to Issues in Progress in 5.0.0 SR6 (Planning) - Target Date Oct. 13th via automation Sep 21, 2021
@jfversluis jfversluis moved this from Issues in Progress to PR Needs Review in 5.0.0 SR6 (Planning) - Target Date Oct. 13th Sep 21, 2021
@jfversluis
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jfversluis jfversluis merged commit a21b36a into xamarin:5.0.0 Oct 8, 2021
5.0.0 SR6 (Planning) - Target Date Oct. 13th automation moved this from PR Needs Review to Done Oct 8, 2021
@VitalyKnyazev
Copy link
Contributor

I am getting new exception on iOS 12.4 after updating to SR6:
Name: NSRangeException Reason: Cannot remove an observer ... for the key path "sublayers" from <CALayer 0x600007b11e80> because it is not registered as an observer.

It happens on EntryRenderer Dispose call at this line:
ClearButton?.Layer?.RemoveObserver(this, new NSString("sublayers"));

@lee-m
Copy link
Contributor

lee-m commented Oct 29, 2021

@rotorgames @jfversluis I'm also seeing the same crash that @VitalyKnyazev reports above in my app after updating to SR6.

AddObserver is documented as returning "An IDisposable object.  Invoke the Dispose method on this object to remove the observer." so maybe we need to store the return value from AddObserver and dispose it instead of calling RemoveObserver?

EDIT: This was on an iPhone 6 running iOS 12.5.5, seems sensitive to the version of iOS being used much like the original issue was.

@jfversluis
Copy link
Member

@VitalyKnyazev @lee-m would you mind opening a new issue for this and if possible attach a small reproduction sample to show the issue? That would be very helpful. If you can do that I will see to prioritize this for the next release

@lee-m
Copy link
Contributor

lee-m commented Oct 29, 2021

@jfversluis @VitalyKnyazev I've opened #14804 for this. I've left the repro steps blank for now whilst I try and distill down a small example that demonstrates the problem but it might be a bit tricky.

@xleon
Copy link

xleon commented Nov 2, 2021

I'm getting this crash too.
It doesn't matter what ClearButtonVisibility is set to. It crashes anyway when disposing (normally when navigating away from the page where the Entry is).
As a workaround I'm overriding Dispose() method with a try/catch in my custom EntryRenderer:

protected override void Dispose(bool disposing)
{
    try
    {
        base.Dispose(disposing);
    }
    catch (Exception)
    {
        // TODO remove this hack on newer version of Xamarin.Forms: https://github.com/xamarin/Xamarin.Forms/pull/14526
    }
}

jfversluis added a commit that referenced this pull request Nov 16, 2021
jfversluis added a commit that referenced this pull request Nov 16, 2021
* Revert "Fix hangs and NREs with Entry Observer (#14859)"

This reverts commit 5188c4f.

* Revert "Refactor observer lifecycle (#14828)"

This reverts commit f95f671.

* Revert "Fix crashing on iOS < 14 if ClearButtonVisibility = WhileEditing in Entry (#14479 #14510) (#14526)"

This reverts commit a21b36a.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often p/iOS 🍎 t/bug 🐛
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants