I'm currently not getting thumbnails for some sites. Wondering how I can debug this. It looks like bugzilla bugs are the most common. str: 1) open start 2) find a site in Top Sites that doesn't have a thumbnail 3) open the site, let it fully render 4) close the tab 5) open start result: thumbnail still not rendered

A lot of pages are not captured for various reasons http://mxr.mozilla.org/mozilla-central/source/browser/metro/base/content/browser-ui.js#828 bugzilla uses the store control: no cache setting, v.mozilla uses https, both of which are 'do not capture' reasons. What other sites have you experienced this with?

The plan to fix this for desktop Firefox is bug 841495. We should make sure the same code can be used in Metro.

Desktop Firefox excludes https only if browser.cache.disk_cache_ssl is set to false. It's true by default. Any reason why you dropped that condition?

(In reply to Dão Gottwald [:dao] from comment #3) > Desktop Firefox excludes https only if browser.cache.disk_cache_ssl is set > to false. It's true by default. Any reason why you dropped that condition? browser.cache.disk_cache_ssl appears not to exist on metro. Afaik it did not exist on old fennec and it has not been added since we forked.

(In reply to :Ally Naaktgeboren from comment #4) > browser.cache.disk_cache_ssl appears not to exist on metro. Afaik it did not > exist on old fennec and it has not been added since we forked. browser.cache.disk_cache_ssl defaults to true for all Gecko applications, including Metro Firefox.

Stealing this bug and morphing it just to handle the HTTPS case. I'll file a separate bug to use background thumbnailing (bug 841495) once it is available.

Comment on attachment 724663 [details] [diff] [review] patch Review of attachment 724663 [details] [diff] [review]: ----------------------------------------------------------------- thank you. r+ with a couple nits ::: browser/metro/base/content/browser-ui.js @@ +93,5 @@ > window.addEventListener("MozPrecisePointer", this, true); > window.addEventListener("MozImprecisePointer", this, true); > > Services.prefs.addObserver("browser.tabs.tabsOnly", this, false); > + Services.prefs.addObserver("browser.cache.disk_cache_ssl", this, false); no removeObserver call? @@ +533,4 @@ > this._updateTabsOnly(); > + } else if (aData == "browser.cache.disk_cache_ssl") { > + this._sslDiskCacheEnabled = Services.prefs.getBoolPref(this.PREF_DISK_CACHE_SSL); > + } This is legal in terms of style, but I find it hard to read. Might I suggest replacing the aData comparisons with a switch instead? case "nsPref:changed": switch(aData) { case: "browser.tabs.tabsonly": .... break; case: "browser.cache.disk_cache_ssl": .... break; }

(In reply to :Ally Naaktgeboren from comment #8) > no removeObserver call? Since BrowserUI lives until shutdown, I prefer to let the observer clean up the observers automatically rather than adding code to do it manually. > This is legal in terms of style, but I find it hard to read. Might I suggest > replacing the aData comparisons with a switch instead? Fixed. https://hg.mozilla.org/integration/mozilla-inbound/rev/adc0079eeb54

(In reply to Matt Brubeck (:mbrubeck) from comment #9) > Since BrowserUI lives until shutdown, I prefer to let the observer clean up > the observers... I meant, I prefer to let the observer *service* clean up the observers.

Does this handle Cache-control: no-store appropriately? See bug 754608 and bug 755996.