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

[iOS] RemoveObserver only if previously was added #14790

Closed
wants to merge 1 commit into from
Closed

Conversation

jsuarezruiz
Copy link
Contributor

@jsuarezruiz jsuarezruiz commented Oct 25, 2021

Description of Change

Avoid possible exception with custom Entry renderer. If create a Custom Renderer will not register the sublayers observer but will try to remove it disposing the renderer. This PR include changes to avoid remove the observer if previously was not added.

Fixes

Cannot remove an observer X for the key path "Y" from Z because it is not registered as an observer.

Issues Resolved

API Changes

None

Platforms Affected

  • iOS

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

PR Checklist

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

@jsuarezruiz jsuarezruiz changed the title [iOS] RemoveObserver only if previously was added. Avoid possible exception… [iOS] RemoveObserver only if previously was added Oct 25, 2021
@tylerjnettleton
Copy link

Do we have an ETA of this being merged?

@tylerjnettleton
Copy link

@jsuarezruiz @jfversluis Tested against PR build 5.0.0.7769 and bug is still reproducible.

Stack trace:

Objective-C exception thrown. Name: NSRangeException Reason: Cannot remove an observer <Syncfusion_SfPdfViewer_XForms_iOS_CustomPageRenderer 0x7ff700f23630> for the key path "sublayers" from <CALayer 0x600007bb2260> because it is not registered as an observer. Native stack trace: 0 CoreFoundation 0x000000010c77abb4 __exceptionPreprocess + 242 1 libobjc.A.dylib 0x000000011aa6dbe7 objc_exception_throw + 48 2 Foundation 0x000000011bf1a062 -[NSObject(NSKeyValueObserverRegistration) _removeObserver:forProperty:] + 689 3 Foundation 0x000000011bf1a4c2 -[NSObject(NSKeyValueObserverRegistration) removeObserver:forKeyPath:] + 129 4 Coach Corner.iOS 0x0000000102853669 xamarin_dyn_objc_msgSend + 217 5 ??? 0x00000001622180fe 0x0 + 5941330174

@jfversluis
Copy link
Member

@tylerjnettleton would you be able to test #14828 ? I see this tries to do the same thing. Didn't notice this one 😅

@lee-m
Copy link
Contributor

lee-m commented Nov 4, 2021

Is this change correct? This bit from the PR description doesn't make sense to me

If create a Custom Renderer will not register the sublayers observer but will try to remove it disposing the renderer

I have a custom Entry renderer in my app that's seeing the same issue and the very first line in that override is calling base.OnElementChanged which is where the observer is being added.

@jfversluis
Copy link
Member

@lee-m to avoid any confusion here. I think this PR and the one you tested try and fix the same thing. I simply did not see this one while I created the other one.

If mine works good in all scenarios I will merge my PR and this one will be abandoned.

If I understand you correctly, my change also works well for you when you are using a custom renderer?

@lee-m
Copy link
Contributor

lee-m commented Nov 4, 2021

@jfversluis Yes your changes work when simply swapping out the NuGet packages for the preview ones with your fix in.

I've spent several days trying to find the root cause and seeing reference to a custom renderer got me thinking if that was it, but then realised the reasoning given behind why that caused it (AddObserver not called) couldn't be true for my case so had me questioning whether the changes in this PR are really fixing anything.

@jfversluis
Copy link
Member

Seeing the response from @tylerjnettleton it doesn't seem to fix it, just waiting for them to confirm so we can move forward on the other fix.

jfversluis added a commit that referenced this pull request Nov 9, 2021
Co-Authored-By: Javier Suárez <javiersuarezruiz@hotmail.com>
jfversluis added a commit that referenced this pull request Nov 10, 2021
* Revert "Refactor observer lifecycle (#14828)"

This reverts commit f95f671.

* Implement fix from Javier (#14790)

Co-Authored-By: Javier Suárez <javiersuarezruiz@hotmail.com>

Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NsRangeException while leaving content page containing custom entry with Xamarin Forms (v5.0.0.2196)
4 participants