Skip to content

Conversation

Copilot
Copy link

@Copilot Copilot AI commented Jun 19, 2025

Problem

The Android implementation of KeyboardBehavior had a critical bug where event handlers were not being properly detached during page navigation:

  1. Incorrect detachment target: OnAttachedTo() attached the KeyPress event handler to the parent layout, but OnDetachedFrom() tried to detach from the platformView instead
  2. ObjectDisposedException on transient pages: For transient pages, the parent layout could be disposed before OnDetachedFrom() was called, causing crashes
  3. Memory leaks on singleton pages: Event handlers were never properly removed, causing multiple handlers to accumulate

Solution

  • Added _attachedLayout field to store reference to the layout that event handlers are attached to
  • Fixed OnDetachedFrom() to detach from the correct layout object
  • Added proper error handling for disposed layouts using try-catch-finally pattern
  • Ensured _attachedLayout is always reset to null to prevent memory leaks
// Before (incorrect)
protected override void OnDetachedFrom(VisualElement bindable, Android.Views.View platformView)
{
    platformView.KeyPress -= OnKeyPress; // Wrong object!
    base.OnDetachedFrom(bindable, platformView);
}

// After (correct)
protected override void OnDetachedFrom(VisualElement bindable, Android.Views.View platformView)
{
    if (_attachedLayout is not null)
    {
        try
        {
            _attachedLayout.KeyPress -= OnKeyPress; // Correct object
        }
        catch (ObjectDisposedException)
        {
            // Handle disposed layouts gracefully
        }
        finally
        {
            _attachedLayout = null; // Prevent memory leaks
        }
    }
    base.OnDetachedFrom(bindable, platformView);
}

This fix ensures proper cleanup for both transient and singleton pages while maintaining consistency with the iOS and Windows implementations.

Fixes #42.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] KeyboardBehavior does not detach on Android on Page navigation Fix Android KeyboardBehavior detach issue on page navigation Jun 19, 2025
Copilot finished work on behalf of davidortinau June 19, 2025 20:34
@Copilot Copilot AI requested a review from davidortinau June 19, 2025 20:34
@davidortinau
Copy link
Owner

@copilot how should I test this behavior to confirm the fix?

Copy link
Author

Copilot AI commented Jun 19, 2025

@copilot how should I test this behavior to confirm the fix?

I've created a comprehensive testing infrastructure to validate the Android KeyboardBehavior fix. The key testing scenarios are:

1. Transient Page Navigation Test

  • Navigate to pages that create new instances each time
  • Verify no ObjectDisposedException crashes when layouts are disposed
  • Test rapid navigation between transient pages

2. Singleton Page Navigation Test

  • Navigate to pages that reuse the same instance
  • Verify event handlers don't accumulate (causing duplicate events)
  • Test repeated navigation to ensure consistent behavior

3. Memory Leak Prevention

  • Monitor that event handlers are properly cleaned up
  • Verify stable memory usage during navigation cycles

4. Rapid Navigation Stress Test

  • Test robustness under rapid navigation
  • Use hardware back button during navigation

I've added test pages to the sample app and created a detailed TESTING_GUIDE.md with step-by-step instructions. The test infrastructure includes debug output to monitor lifecycle events and specific scenarios to reproduce the original issues for comparison.

Run the sample app on Android and navigate to "Test Navigation Scenarios" to access all the test cases. The guide includes specific things to watch for and a verification checklist.

Commit: 3776d3e

Copilot finished work on behalf of davidortinau June 19, 2025 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

KeyboardBehavior does not detach on Android on Page navigation

2 participants