Closed Bug 1192810 Opened 9 years ago Closed 9 years ago

Home panel flashes before content loads on cold boot


(Firefox for iOS :: Home screen, defect)

Not set



Tracking Status
fxios + ---


(Reporter: sleroux, Assigned: sleroux)


(Keywords: regression, reproducible, Whiteboard: [1.0?])


(1 file, 1 obsolete file)


1. Visit
2. Hard close the app
3. Open app


Should see white content while page is loading in


Home panel/top sites flashes in before web content appears
tracking-fxios: --- → ?
No this looks like a regression. I don't remember the top sites panel flashing in before rendering content.
I've noticed this for awhile, but forgot to file a bug for it. I think this happened after bug 1182274, which changed the way we detect URL changes on the webview. Previously, we used didCommitNavigation to look for URL changes, but that wasn't reliable since it wasn't fired for all cases. Using KVO to monitor the web view's URL is more reliable -- so reliable, in fact, that we we catch the very brief period between the web view being created with the initial about:home URL and setting the follow-up URL!

Restored tabs are created without specifying a request [1], so they'll use the default about:home request before loading the sessionData. The fix might be as simple as skipping the default request if "restoring" is true.

I shall take once done with presentación
Bryan: take it when you're done with the other two+ bugs :)
Hardware: Other → All
Whiteboard: [1.0?]
Assignee: nobody → bmunar
Assignee: bmunar → sleroux
After some digging, it looks like the reason the home pane view controller flashes is because we're resetting the page's push state to the home URL first, then navigating to their latest page:

The first url is usually always about:home which, when replaceState gets called, loads into the web view which in turn updates WKWebView's URL property which triggers the KVO observer. I don't know what this is supposed to do but is there something we can do about this?
Flags: needinfo?(bnicholson)
We have to call replaceState (rather than pushState) with the first URL to be restored since that removes the SessionRestore.html loader page from the back/forward history. We then have to use pushState in order of the history entries, so I'm not sure we can avoid this on the JS end.

One way to fix this would be to delay attaching the KVO observers until the restore has happened, which kind of makes sense anyway since we don't want the restore to fire these events. Not a super simple fix like I was hoping, though.
Flags: needinfo?(bnicholson)
Found a simple workaround for this. In the KVO observer, we check to see if the previous URL was session restore. If so, skip the reader mode/home panel checks.
Attachment #8647104 - Flags: review?(bnicholson)
Comment on attachment 8647104 [details] [review]

Heh, this is the approach I was going to suggest first, but I don't think it will work in the case where about:home is actually the active restored URL. That is: create a new tab (which opens about:home), then close/reopen the browser. I think that will break the home panel appearing since the previous page was the session restore URL. Please reflag review if I'm wrong!
Attachment #8647104 - Flags: review?(bnicholson) → review-
Ya looks like this breaks in this scenario - I'll see what else I can do :(
Comment on attachment 8647104 [details] [review]

I've added a quick/dirty workaround for the restore->home issue

Attachment #8647104 - Flags: review- → review?(bnicholson)
Comment on attachment 8647104 [details] [review]

Well, this is kind of gross, but I guess it's good enough for now :)
Attachment #8647104 - Flags: review?(bnicholson) → review+
Attachment #8647104 - Attachment is obsolete: true
Okay - I implemented a proper way of doing this instead of a lazy hack. I now delay KVO registration until restoration is complete.
Attachment #8647526 - Flags: review?(bnicholson)
Comment on attachment 8647526 [details] [review]

I think this looks OK for restored tabs, but this won't work for new tabs that aren't restored. For example, try creating a tab the navigation to a new URL -- the home panel won't disappear.

Maybe add a restoring boolean to Browser that we set to true at the end of this if condition [1], then set to false after firing didRestoreSessionForBrowser? Then you could do if !browser.restoring { webView.addObserver(...) } in didCreateWebView. I think that might also allow you to drop the try/catch if you wanted since it would be safe to call the webView.removeObserver(...) as long browser.restoring is false.

Attachment #8647526 - Flags: review?(bnicholson) → feedback+
(In reply to Brian Nicholson (:bnicholson) from comment #14)
> For example, try creating a tab the navigation to a new URL

Typos. This should have been: "For example, try creating a tab, then navigating to a new URL...".
Ya that would work. Bah and I was trying to avoid adding more boolean state.
Comment on attachment 8647526 [details] [review]

Updated with the restoration flag and it seems to be working fine for me (new tab + restoration)
Attachment #8647526 - Flags: review?(bnicholson)
Comment on attachment 8647526 [details] [review]

Nice, thanks for taking this!
Attachment #8647526 - Flags: review?(bnicholson)
Attachment #8647526 - Flags: review+
Attachment #8647526 - Flags: feedback+
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.