Skip to content

Conversation

@d4r1091
Copy link
Member

@d4r1091 d4r1091 commented Dec 23, 2025

MOB-4049

Context

App crashed on iOS < 16 when trying to dequeue NTPHeader cell with an empty identifier.

Approach

The root cause

NTPHeaderViewModel.isEnabled returned true on all iOS versions, causing the section to be added to the collection view even on iOS < 16 where NTPHeader is unavailable.

Solution

Modified NTPHeaderViewModel.isEnabled to return false on iOS < 16. This makes shouldShow evaluate to false, filtering the section out before dequeue is attempted. The header section is completely excluded on devices running iOS < 16.

Other

Before merging

Checklist

  • I performed some relevant testing on a real device and/or simulator for both iPhone and iPad

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Behavior Change

On iOS < 16 the header is now completely disabled regardless of feature flags. Confirm this aligns with product requirements and that no dependent logic assumes the header exists on older iOS versions.

var isEnabled: Bool {
    if #available(iOS 16.0, *) {
        return AISearchMVPExperiment.isEnabled
    }
    return false
}
Test Coverage

Consider adding unit/UI tests to validate that isEnabled returns true only on iOS >= 16 when the experiment is enabled, and false otherwise, to prevent regressions.

var isEnabled: Bool {
    if #available(iOS 16.0, *) {
        return AISearchMVPExperiment.isEnabled
    }
    return false
}

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Add compile-time availability guard

Guard the feature flag access behind a compile-time check to prevent referencing
unavailable symbols on older SDKs. This avoids potential runtime or compile-time
issues when building with deployment targets below iOS 16.

firefox-ios/Client/Ecosia/UI/NTP/Header/NTPHeaderViewModel.swift [72-77]

 var isEnabled: Bool {
+#if compiler(>=5.7)
     if #available(iOS 16.0, *) {
         return AISearchMVPExperiment.isEnabled
     }
+#endif
     return false
 }
Suggestion importance[1-10]: 4

__

Why: The existing code already uses a runtime availability check and likely compiles fine; adding a compiler version guard offers marginal additional safety. The 'existing_code' matches the new hunk lines, and the improved code reflects the suggested change accurately.

Low

@d4r1091
Copy link
Member Author

d4r1091 commented Dec 23, 2025

Merging this myself given the availability circumstances.

@d4r1091 d4r1091 merged commit e12f837 into patch/fix-ios-15-crash-for-ai-shortcuts Dec 23, 2025
2 of 3 checks passed
@d4r1091 d4r1091 deleted the dc-mob-4049-crash-on-ios15 branch December 23, 2025 15:27
d4r1091 added a commit that referenced this pull request Dec 29, 2025
Squashed commit of the following:
commit 43fbd03
Author: Dario Carlomagno <dario.carlomagno@ecosia.org>
Date:   Tue Dec 23 17:23:05 2025 +0100

    Update version to 11.5.3 (#991)

commit e12f837
Author: Dario Carlomagno <dario.carlomagno@ecosia.org>
Date:   Tue Dec 23 16:27:29 2025 +0100

    [MOB-4049] Fix on the header VM for iOS<16 (#990)

commit 1d2b8bd
Author: Dario Carlomagno <dario.carlomagno@ecosia.org>
Date:   Tue Dec 23 16:20:06 2025 +0100

    [IOS-15] Include `patch/` to behave same as `main`
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