Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not fire onExitFullscreen when configuration changes are being applied #5

Conversation

Silic0nS0ldier
Copy link
Contributor

@Silic0nS0ldier Silic0nS0ldier commented Oct 16, 2024

By default Android will recreate activities (running their lifecycle methods in the process) when any of the following changes;

  • Color capabilities of the screen (colorMode).
  • Display density, such as scaling changes or moving the activity to a different screen (density)
  • Font scaling factor (fontScale)
  • Font weight (fontWeightAdjustment)
  • Grammatical gender of the active language (grammaticalGender)
  • Keyboard type, like a keyboard being plugged in (keyboard)
  • Keyboard accessibility, such as the user revealing a builtin hardware keyboard (keyboardHidden)
  • Layout direction, LTR and RTL (layoutDirection)
  • Locale (locale)
  • IMSI mobile country code (mcc)
  • IMSI mobile network code (mnc)
  • Navigation type, like trackball to d-pad (navigation)
  • Screen orientation (orientation)
  • Screen layout, possibly triggered by a different display becoming active (screenLayout)
  • Screen size as perceived by the app (screenSize)
  • Actual screen size (smallestScreenSize)
  • Touchscreen (touchscreen)
  • UI mode, such as when a device is placed in a dock, connected to a car, or night mode activation (uiMode)

Out of all those, orientation and screenSize are the most likely to change (they'll both change when rotating the device).

The trouble is detection of leaving full screen mode is currently handled by onDestroy(), which Android will invoke when recreating the activity. As a result onExitFullscreen is called too early which means;

  • Internal state is out of sync, isFullscreen is set to false.
  • If the video was muted before entering full screen (default), it is muted.
  • If autoplay is disabled, the video is paused.
  • The player is added back to playerView too early.

There are 2 ways of solving this issue;

  1. Add each change type to android:configChanges in AndroidManifest.xml and handle changes ourselves.
  2. Use isChangingConfigurations() to determine why lifecycle methods like onDestroy are being called, and only run onExitFullscreen when false.

This PR implements (2) for the following reasons;

  • It is a simple 3 line change (1 of which is just an indent)
  • Any configuration dependent behaviours and visuals will be automatically refreshed
  • No need to update if/when a new configuration types is introduced
  • isChangingConfigurations() is a stable API, available since API level 11 (Android 3.0).
    Granted, android:configChanges has been around for even longer.

This change has been tested on a Pixel 8 with Android 14.

@haileyok
Copy link
Collaborator

Tested this locally! Sorry for the delay in seeing this (got to clean up my email...), appreciate all the tweaking you've done on the Android side 🙏

@haileyok haileyok merged commit fcaea07 into bluesky-social:main Oct 25, 2024
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.

2 participants