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

Bug 1215669 - Queue tab screenshots if BVC isn't visible #1206

Closed

Conversation

thebnich
Copy link
Contributor

I think the issue here is that WKWebView doesn't render if its view controller isn't visible (presumably as an optimization). This fixes the problem by waiting until BVC is visible, then trying again.

I spent awhile trying to figure out a good way to determine view controller visibility; in particular, I tried self.navigationController?.visibleViewController === self and isViewLoaded() && view.window != nil. Those mostly work, but I found I could trigger some edge casey false positives if the screenshotting fires during the tab tray -> BVC transition (these conditions all return true, yet the screenshot was still blank). I bit the bullet and tacked on a viewIsVisible boolean that I just keep track of manually.

The ScreenshotHelper protocol is an abstraction artifact from when we
needed to take screenshots from TabTrayController. Since we don't do
this anymore, we can remove this layer of abstraction.
@thebnich
Copy link
Contributor Author

@jhugman Updated the PR. I moved the ScreenshotHelper implementation into ScreenshotHelper.swift, along with the other state and methods. Is this what you had in mind?

thebnich added a commit that referenced this pull request Nov 2, 2015
Bug 1215669 - Queue tab screenshots if BVC isn't visible
@thebnich
Copy link
Contributor Author

thebnich commented Nov 2, 2015

Landed fa0fd4e.

@thebnich thebnich closed this Nov 2, 2015
@thebnich thebnich deleted the background-thumbnails branch November 2, 2015 20:50
isabelrios pushed a commit to isabelrios/firefox-ios that referenced this pull request Feb 19, 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.

None yet

2 participants