645509 - Chrome tabs should be allowed to have favicons too

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, defect)

Description

[Jonathan Protzenko [:protz]]

Attachment: Just add and remove the right event handlers from chrome tabs too

Currently, the code from specialTabs.js has a DOMLinkHandler class that reacts upon seeing a <link> element pass by, and updates the favicon of the tab accordingly. Unfortunately, this is just for content tabs. Chrome tabs might want to have their favicon too! In my case, my "compose in a tab" thing wants to have a nice favicon too, so the attached patch does just that.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 522222 [details] [diff] [review]
Just add and remove the right event handlers from chrome tabs too

Standard8 seems like the right person to review this code.

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 522222 [details] [diff] [review]
Just add and remove the right event handlers from chrome tabs too

I've not explicitly tested this, but the code looks fine. One nit:

>+      aTab.browser.addEventListener("DOMLinkAdded", DOMLinkHandler, false);
>+
>+
>       aTab.browser.setAttribute("id", "chromeTabBrowser" + this.lastBrowserId);

only one blank line necessary here.

Comment 3

[Jonathan Protzenko [:protz]]

http://hg.mozilla.org/comm-central/rev/7fd054a760dd

(sorry, I forgot to address the nit, tell me if you want one more commit for this)