Selectively display the globe favicon placeholder when favicon fetches take long enough to be noticeable

VERIFIED FIXED in Firefox 27

Status

()

defect
VERIFIED FIXED
6 years ago
3 years ago

People

(Reporter: ibarlow, Assigned: rnewman)

Tracking

(Blocks 1 bug, {polish})

27 Branch
Firefox 29
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox27 verified, firefox28 verified, firefox29 verified, fennec27+)

Details

Attachments

(1 attachment)

It looks like some of the work that happened in bug 923218 had an effect on more than just the home page lists. The *URL bar* favicon behaviour seems to have changed too. 

We are now often seeing a blank void while we wait for a favicon to load, instead of the previous (and more desirable) behaviour where we placed a globe while waiting for the favicon. 

Let's use this bug to restore the original behaviour to the URL bar.
Blocks: 923218
Yeah, that was deliberate, but we probably overshot. (We did try to get this right, though: Bug 923218 Comment 6! :P)

The previous behavior was:

* Show the spinner.
* Hide the spinner, replace it with a blank spot.
* Show the globe.
* A few milliseconds later, show the real favicon.

That looked really janky and busy when the favicon was ready for display, and still kinda janky when the favicon took less than two seconds to load.

We fixed this in Bug 923218: instead of showing the globe *then* showing the favicon, we only show the globe if the favicon fetch failed. Typically that's quick, but there's still a gap between the spinner and the icon, so if the favicon takes a while to load it looks weird in a different way.

mfinkle and I were chatting about this today. The ideal behavior would be to do something like this:

* Overlay the spinner on the globe.
* Fade out the spinner as the page load completes, revealing the globe.
* Fade in the real favicon when it arrives (which ideally will be immediately as the spinner disappears).

Sriram probably has some input on this from a drawing perspective…
Component: General → Theme and Visual Design
OS: Mac OS X → Android
Hardware: x86 → All
Version: Firefox 26 → Firefox 27
Aaah, I see what happened in that bug.

I filed it about the URL bar ("URL bar" in Bug 923218 Comment 0), and you thought I was talking about the home page lists ("slots" in Bug 923218 Comment 3). Sorry, I should have been clearer about which bit of the UI we were tweaking :D

On the plus side, now we have the opportunity to make it even better!
(In reply to Richard Newman [:rnewman] from comment #1)
> mfinkle and I were chatting about this today. The ideal behavior would be to
> do something like this:
> 
> * Overlay the spinner on the globe.
> * Fade out the spinner as the page load completes, revealing the globe.
> * Fade in the real favicon when it arrives (which ideally will be
> immediately as the spinner disappears).

IMO, the gist of this bug is that there should never be a void in the toolbar. The animation part is an extra, I'd discuss/fix it in a separate bug.
tracking-fennec: --- → ?
Assignee: nobody → rnewman
tracking-fennec: ? → 27+
Status: NEW → ASSIGNED
Per discussion with Ian on IRC: the short-term fix for this bug would be to undo Bug 923218. That sucks, too, so we're not going to do it.

Instead, I'm going to file a bug to preload about:home's favicon, so it loads instantly on first run (no silly void before the Fennec/Firefox icon loads). That's a big part of the ugliness, and what we do now is stupid, so let's just fix that.

(If we do the special-page fetching logic right, then this'll also buy us other favicon speedups.)

That'll leave the only bad experience being a void when loading a site's favicon for the first time (from network or disk).

I plan to leave this bug open to address that; the solution will probably be to have an additional favicon cache callback method that tells the caller whether they're going to have to wait for a disk/network fetch; in that method we can show the globe, so the void will only exist for an instant (long enough to check the cache), and then we'll either show globe then icon, or just wait another instant for the real icon to show up.

Re-flagging as tracking 'cos I don't know if we'd want to take that non-trivial work for near-beta or early-beta.

Speak now or forever hold your peace.
Blocks: 926139
No longer blocks: new-about-home
tracking-fennec: 27+ → ?
Filed as Bug 941868.
Keywords: polish
Summary: Put the globe favicon back into the URL bar → Selectively display the globe favicon placeholder when favicon fetches take long enough to be noticeable
Assignee: rnewman → nobody
Status: ASSIGNED → NEW
Last triage meeting had a lot of folks suggesting the old "show globe, then show favicon" was better than the current behavior. Let's get a better plan.
Assignee: nobody → rnewman
tracking-fennec: ? → 27+
(In reply to Mark Finkle (:mfinkle) from comment #6)
> Last triage meeting had a lot of folks suggesting the old "show globe, then
> show favicon" was better than the current behavior. Let's get a better plan.

You want the easy path (flickering globe in most cases), or the more difficult path (show the globe for non-cached icons)?
Status: NEW → ASSIGNED
Flags: needinfo?(mark.finkle)
Re-ping? I'm inclined to take the slightly more difficult route here, just because the old behavior was ugly enough that we already tried to fix it once.
Sure. Let's start exploring the more difficult route. If you could see my face, you would know the extent of my displeasure in adding more complexity for favicons.

There has got to be some simple, elegant way forward here.
Flags: needinfo?(mark.finkle)
Howzat? Shows the globe unless the favicon is cached, in which case we don't show the globe, and the favicon appears immediately. Looks good, though on my device the favicon loads are so quick compared to page loads that you only see the globe for 100ms anyway.
Attachment #8349193 - Flags: review?(mark.finkle)
Comment on attachment 8349193 [details] [diff] [review]
Selectively display the globe favicon placeholder when favicon fetches take long enough to be noticeable.

>diff --git a/mobile/android/base/toolbar/BrowserToolbar.java b/mobile/android/base/toolbar/BrowserToolbar.java

>+    public void showDefaultFavicon() {
>+        mFavicon.setImageResource(R.drawable.favicon);
>+    }

Should we reset mLastFavicon in here? We use mLastFavicon for some optimizations in setFavicon.
Attachment #8349193 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/integration/fx-team/rev/7ef525f0c865

If this sticks, will request uplift.
Flags: needinfo?(rnewman)
Target Milestone: --- → Firefox 29
https://hg.mozilla.org/mozilla-central/rev/7ef525f0c865
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Duplicate of this bug: 927186
Comment on attachment 8349193 [details] [diff] [review]
Selectively display the globe favicon placeholder when favicon fetches take long enough to be noticeable.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  Multiple aeons of favicon improvements.

User impact if declined: 
  Users will see a blank spot in the URL bar until a network favicon is loaded. This patch reintroduces the grey globe icon unless a page's favicon is already cached.

Testing completed (on m-c, etc.): 
  Been baking for a while with no observed problems to my knowledge.

Risk to taking this patch (and alternatives if risky): 
  One of the least risky favicon changes in recent months :P

String or IDL/UUID changes made by this patch:
  None.
Attachment #8349193 - Flags: approval-mozilla-beta?
Attachment #8349193 - Flags: approval-mozilla-aurora?
Flags: needinfo?(rnewman)
Attachment #8349193 - Flags: approval-mozilla-beta?
Attachment #8349193 - Flags: approval-mozilla-beta+
Attachment #8349193 - Flags: approval-mozilla-aurora?
Attachment #8349193 - Flags: approval-mozilla-aurora+
Keywords: verifyme
Verified as fixed in builds:
- FF 27;
- 28 Beta 4;
- Aurora 29.0a2 (2014-02-23);
Device: Asus Transformer Tab (Android 4.0.3).
You need to log in before you can comment on or make changes to this bug.