Closed Bug 674794 Opened 10 years ago Closed 10 years ago

favicons of app tabs aren't updated on Panorama

Categories

(Firefox Graveyard :: Panorama, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 9

People

(Reporter: reuben, Assigned: raymondlee)

References

Details

Attachments

(4 files, 2 obsolete files)

For example, if you turn on Gmail Labs' "Unread message icon" feature, the favicon on Panorama will not change unless you close and reopen the tab.
This is an Add-on for Firefox? If yes, can you provide a link to test this?
Thx.
No, Panorama a.k.a. TabCandy is a built-in feature since 4.0 (?).
Just click the "Group your tabs" button on the top right of the window.
I'm referring in comment1 about Gmail Labs or is it a feature of gmail that can be activated?
(In reply to comment #3)
> I'm referring in comment1 about Gmail Labs or is it a feature of gmail that
> can be activated?

(comment #0)
> Gmail Labs' "Unread message icon" feature

Look for "Unread message icon" on your Gmail Labs settings.
Reproducible for me on  Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0a1) Gecko/20110728 Firefox/8.0a1  
and also on Mozilla/5.0 (Windows NT 6.1; rv:8.0a1) Gecko/20110728 Firefox/8.0a1

Changing platform to All since is reproducible on Windows 7 as well.

Setting resolution to NEW.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Mac OS X → All
This works for me.

When an attribue (e.g. title, icon url, etc)  is modified, the fav icon, title and thumbnail would be updated (see TabItems_update()) immediately or after a delay if there are other tab items waiting to be updated or the tab item itself has just been updated less than 200 million seconds ago.

Reporter: please try
1. open a tab with gmail
2. go into Panorama
3. open a new window or use other email client to send an email your emil account
4. wait a few minutes and the fav icon and title should get updated.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WORKSFORME
(In reply to Raymond Lee [:raymondlee] from comment #6)
> 
> Reporter: please try
> 1. open a tab with gmail
> 2. go into Panorama
> 3. open a new window or use other email client to send an email your emil
> account
> 4. wait a few minutes and the fav icon and title should get updated.
>

I can still reproduce this, and I don't think I should have to wait, why can't it be instantaneous?
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Attached video Video
> I can still reproduce this, and I don't think I should have to wait, why
> can't it be instantaneous?

It works fine for me.  Please see the attached video.

Two google mail tabs with the same account in each window.  One in Panorama and one isn't.  The fav and title are updated almost at the same time.
Oh, I completely forgot to mention, it only happens on app tabs.
Summary: favicons aren't updated on Panorama → favicons of app tabs aren't updated on Panorama
Attached video video for app tabs
(In reply to Reuben Morais [:reuben] from comment #9)
> Oh, I completely forgot to mention, it only happens on app tabs.

Still works for me.
Attached video video
Attachment #554377 - Attachment mime type: application/octet-stream → video/quicktime
Attached patch v1 (obsolete) — Splinter Review
-    // use the tab image if it doesn't start with http e.g. data:image/png, chrome://
-    if (tab.image && !(/^https?:/.test(tab.image)))
-      url = tab.image;
-    else
+    if (tab.image) {
+      // if starts with http/https, fetch icon from favicon service via the moz-anno protocal
+      if (/^https?:/.test(tab.image))
+        url = "moz-anno:favicon:" + tab.image;

This would get the icon from fav icon service using the icon url

+      else
+        url = tab.image;
+    } else {
       url = gFavIconService.getFaviconImageForPage(tab.linkedBrowser.currentURI).spec;

This doesn't return the latest fav icon by url so we use "moz-anno:favicon:" above

+    }
Attachment #554519 - Flags: review?(tim.taubert)
Assignee: nobody → raymond
Comment on attachment 554519 [details] [diff] [review]
v1

Review of attachment 554519 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, thanks!

r=me with the one line fixed.

::: browser/base/content/tabview/ui.js
@@ +1569,5 @@
>  
> +    if (tab.image) {
> +      // if starts with http/https, fetch icon from favicon service via the moz-anno protocal
> +      if (/^https?:/.test(tab.image))
> +        url = "moz-anno:favicon:" + tab.image;

Please use "gFavIconService.getFaviconLinkForIcon(gWindow.makeURI(tab.image)).spec" here.
Attachment #554519 - Flags: review?(tim.taubert) → review+
Attached patch v2 (obsolete) — Splinter Review
(In reply to Tim Taubert [:ttaubert] from comment #13)
> Please use
> "gFavIconService.getFaviconLinkForIcon(gWindow.makeURI(tab.image)).spec"
> here.

Done.

I've updated the test and request a review again.
Attachment #554519 - Attachment is obsolete: true
Attachment #554832 - Flags: review?(tim.taubert)
Comment on attachment 554832 [details] [diff] [review]
v2

Review of attachment 554832 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

r=me with the test fix.

::: browser/base/content/test/tabview/browser_tabview_bug600645.js
@@ +40,5 @@
>      // fired, a delay is used here to avoid the test code run before the browser 
>      // code.
>      executeSoon(function() {
> +      ok(/^moz-anno:favicon:/.test($icon.attr("src")), 
> +         "The icon url starts with moz-anno:favicon so the default fav icon would be displayed");

As noted in https://developer.mozilla.org/en/nsIFaviconService#getFaviconLinkForIcon%28%29 the moz-anno protocol handler automatically redirects to the default favIcon if the given url is invalid. This isn't really obvious and the test doesn't check if the default favIcon is displayed...

Please add a comment about the fallback and compare the actual favIcon with the expected one.
Attachment #554832 - Flags: review?(tim.taubert) → review+
(In reply to Tim Taubert [:ttaubert] from comment #15)
> As noted in
> https://developer.mozilla.org/en/
> nsIFaviconService#getFaviconLinkForIcon%28%29 the moz-anno protocol handler
> automatically redirects to the default favIcon if the given url is invalid.
> This isn't really obvious and the test doesn't check if the default favIcon
> is displayed...
> 
> Please add a comment about the fallback and compare the actual favIcon with
> the expected one.

I am not sure how we can compare the actual favicon with the expected one (default one).  Since both faviconService.getFaviconData() and faviconService.getFaviconDataAsDataURL() throws an error for the invalid url (moz-anno:favicon:http://mochi.test:8888/browser/browser/base/content/test/tabview/favicon.ico)
(In reply to Raymond Lee [:raymondlee] from comment #16)
> I am not sure how we can compare the actual favicon with the expected one
> (default one).  Since both faviconService.getFaviconData() and
> faviconService.getFaviconDataAsDataURL() throws an error for the invalid url
> (moz-anno:favicon:http://mochi.test:8888/browser/browser/base/content/test/
> tabview/favicon.ico)

Ok, well I'd say then let's just check that faviconService.getFaviconDataAsDataURL() throws an error and add a nice comment that we expect this error and why we do this.
(In reply to Tim Taubert [:ttaubert] from comment #17)
> Ok, well I'd say then let's just check that
> faviconService.getFaviconDataAsDataURL() throws an error and add a nice
> comment that we expect this error and why we do this.

Done
Attachment #554832 - Attachment is obsolete: true
Keywords: checkin-needed
http://hg.mozilla.org/integration/fx-team/rev/aa126a9aebbd
Keywords: checkin-needed
Hardware: x86 → All
Whiteboard: [fixed-in-fx-team]
Version: 7 Branch → Trunk
Comment on attachment 555100 [details] [diff] [review]
[checked-in] Patch for checkin

http://hg.mozilla.org/mozilla-central/rev/aa126a9aebbd
Attachment #555100 - Attachment description: Patch for checkin → [checked-in] Patch for checkin
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 9
Duplicate of this bug: 682720
Setting resolution to Verified Fixed on Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0 beta 2

The favicon is updating accordingly when new mails are arriving, in Panorama and also in normal mode.
Status: RESOLVED → VERIFIED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.