Closed Bug 1182488 Opened 9 years ago Closed 9 years ago

Changing theme color in ReaderView crashes the app

Categories

(Firefox for iOS :: Reader View, defect)

Other
iOS
defect
Not set
critical

Tracking

()

RESOLVED WORKSFORME
Tracking Status
fxios + ---

People

(Reporter: sleroux, Assigned: mosbahmohamed05)

References

Details

(Keywords: crash)

Attachments

(2 files)

47 bytes, text/x-github-pull-request
sleroux
: review+
sleroux
: feedback+
Details | Review
47 bytes, text/x-github-pull-request
sleroux
: review+
Details | Review
STR:

1. Navigate to https://context.newamerica.org/home-sweet-what-5f68516d157d
2. Turn on Reader Mode by tapping the reader button in the URL bar
3. Tap the Font/theme menu button
4. Tap the 'Light' theme

Expected:

Should restyle the reader view to light theme

Actual:

Crashes.
Severity: major → critical
tracking-fxios: --- → ?
Keywords: crash
I can't recreate this? what simulator did you use?
I wasn't able to produce this on the simulator but I saw it on dev builds. As of Aurora 23 I'm not able to recreate either. I'm going to close as WFM.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WORKSFORME
I'm reproducing on latest dev build iOS simulator 8.4.
[Debug] [Browser.swift:223] 
fatal error: unexpectedly found nil while unwrapping an Optional value
Ah okay - reopening for investigation. Thanks for finding it!
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Attached file Pull request
Attachment #8632749 - Flags: review?(sleroux)
Assignee: nobody → mosbahmohamed05
Hi Mosbah,

Thanks for the contribution! The patch you submitted fixes the problem but I think there's a more fundamental issue here since the helperManager that we're explicitly unwrapping 'should' exist since without it we don't have any of the reader mode functionality. I looks to me the issue might lie in the createWebview method in Browser.swift where we don't instantiate the HelperManager if the webview is nil. It might be related to the way we're 'restoring' our tabs whenever the user comes back to the app. Interested in diving a bit deeper on this one?
Attachment #8632749 - Flags: feedback+
Hi Stephan,

That's what I was thinking too, I'll give it a look.
Status update?
Apparently this is a post-effect crash using anything in the bar...
I don't know what was the exact scenario but I can't reproduce anymore ..
Moshbah's patch should happen anyway. We should never force unwrap anything.
Attached file Pull request
Attachment #8634046 - Flags: review?(sarentz)
Comment on attachment 8634046 [details] [review]
Pull request

Sounds good - I'll merge in the optional unwrap patch and leave this bug open for the nil HelperManager issue since we've already closed a bunch of duplicates to this bug for this issue.
Attachment #8634046 - Flags: review?(sarentz) → review+
Merged in optional unwrap patch. Thanks for the contribution Mosbah!
Attachment #8632749 - Flags: review?(sleroux) → review+
Follow-up?
Flags: needinfo?(sleroux)
Can't seem to reproduce this anymore. Will mark as WORKSFORME and we can reopen in the future if we see it.
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Flags: needinfo?(sleroux)
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: