fenix icon indicating copy to clipboard operation
fenix copied to clipboard

For 25980: Upgrade navigation, lifecycle, fragment and core to latest versions.

Open mcarare opened this issue 2 years ago • 4 comments

Pull Request checklist

  • [x] Tests: This PR includes thorough tests or an explanation of why it does not
  • [x] Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
  • [x] Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

To download an APK when reviewing a PR:

  1. click on Show All Checks,
  2. click Details next to "Taskcluster (pull_request)" after it appears and then finishes with a green checkmark,
  3. click on the "Fenix - assemble" task, then click "Run Artifacts".
  4. the APK links should be on the left side of the screen, named for each CPU architecture

Fixes #25980 Fixes #25980

mcarare avatar Jul 12 '22 06:07 mcarare

Hey there! I'm sorry to jump in this PR. I'm just rebasing it because https://github.com/mozilla-mobile/fenix/pull/26073 reached main. It renamed the expected check from pr-complete to complete-pr. This impacts mergify and it may prevent you from merging this PR. That's why the rebase is needed. Let me know if you have any questions.

JohanLorenzo avatar Jul 20 '22 14:07 JohanLorenzo

I am gonna flag myself as a reviewer here since I am quite familiar with the changes since I tried to upgrade navigation to utilize navigation-compose for some MR work and ran into this loop of having to upgrade everything in https://github.com/mozilla-mobile/fenix/pull/26455, and sadly didn't know we already had this PR.

gabrielluong avatar Aug 17 '22 15:08 gabrielluong

Same issue as the Focus https://github.com/mozilla-mobile/focus-android/pull/7379#issuecomment-1221094720, Locat logs. I rebased on top of https://github.com/mozilla-mobile/android-components/pull/12501/.

Tried to load cnn.com and the app crashed 💥 .

2022-08-21 20:40:11.590 17828-17855/org.mozilla.firefox D/CompatibilityChangeReporter: Compat change id reported: 136069189; UID 10338; state: ENABLED
2022-08-21 20:40:11.591 17828-17855/org.mozilla.firefox W/System.err: java.lang.SecurityException: To use the sampling rate of 0 microseconds, app needs to declare the normal permission HIGH_SAMPLING_RATE_SENSORS.
2022-08-21 20:40:11.591 17828-17855/org.mozilla.firefox W/System.err:     at android.hardware.SystemSensorManager$BaseEventQueue.enableSensor(SystemSensorManager.java:792)
2022-08-21 20:40:11.591 17828-17855/org.mozilla.firefox W/System.err:     at android.hardware.SystemSensorManager$BaseEventQueue.addSensor(SystemSensorManager.java:712)
2022-08-21 20:40:11.591 17828-17855/org.mozilla.firefox W/System.err:     at android.hardware.SystemSensorManager.registerListenerImpl(SystemSensorManager.java:209)
2022-08-21 20:40:11.591 17828-17855/org.mozilla.firefox W/System.err:     at android.hardware.SensorManager.registerListener(SensorManager.java:832)
2022-08-21 20:40:11.591 17828-17855/org.mozilla.firefox W/System.err:     at android.hardware.SensorManager.registerListener(SensorManager.java:739)
2022-08-21 20:40:11.591 17828-17855/org.mozilla.firefox W/System.err:     at org.mozilla.gecko.GeckoAppShell.enableSensor(GeckoAppShell.java:636)
2022-08-21 20:40:11.591 17828-17855/org.mozilla.firefox W/System.err:     at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method)
2022-08-21 20:40:11.591 17828-17855/org.mozilla.firefox W/System.err:     at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:677)
2022-08-21 20:40:11.618 17828-17855/org.mozilla.firefox W/google-breakpad: ExceptionHandler::GenerateDump cloned child 

Amejia481 avatar Aug 22 '22 00:08 Amejia481

The app also crashes on main, with Pixel 4/ Android 13 when opening cnn.com.

LE: https://bugzilla.mozilla.org/show_bug.cgi?id=1780557

mcarare avatar Aug 22 '22 15:08 mcarare

@mcarare It should be noted that updating androidx.core to 1.8.0 also fixes #26674 and #26808 as I mentioned in #27259 (didn't notice this issue updated core... 😅).

For reference, see this bug that was fixed in core 1.8.0-alpha04: https://issuetracker.google.com/issues/180881870

fice-t avatar Oct 04 '22 21:10 fice-t

@jonalmeida just a heads up: after your review and rebasing, I added this commit (so you can review it if needed).

mcarare avatar Oct 10 '22 11:10 mcarare

@jonalmeida just a heads up: after your review and rebasing, I added this commit (so you can review it if needed).

All good - feel free to still land it. :)

jonalmeida avatar Oct 10 '22 18:10 jonalmeida