Welcome to the Splat alpha!

Very weird. We have the same HTML linking to the favicon on both pages.

I think this happens only when our Backbone routers get involved.

• Status:

  +	Confirmed

This definitely seems to be a bug in Firefox. Other browsers handle this correctly.

• Status:

  -	Confirmed
  +	ThirdParty

Apparently Firefox by default limits favicon sizes to 1KB - see the browser.chrome.image_icon.max_size setting (and https://bugzilla.mozilla.org/show_bug.cgi?id=523646).

Review Board's favicon is 17KB, and I've just checked that it gets displayed when I bump the max_size to 64KB.

Huh, interesting. I wonder if there's a way to crush the .ico files to be smaller. If not, we should probably just cut the cord to the past and switch to the .png versions only.

• Status:

  -	ThirdParty
  +	New

• Tags:

  +	EasyFix
  +	Component-WebUI

Bumping browser.chrome.image_icon.max_size and following the STR in the main comment does not fix the issue for me. I also have a feeling this is a Firefox bug.

Removing EasyFix for now - I don't think crushing the size of the favicon is going to fix this.

• Tags:

  -	EasyFix

I ran across this issue and found it a little annoying so I thought I would poke at it a bit.  From Googling it appears it is indeed a Firefox bug:

https://bugzilla.mozilla.org/show_bug.cgi?id=519028

There is useful discussion and solutions at:

http://stackoverflow.com/questions/2409759/firefox-3-6-9-drops-favicon-when-changing-window-location


I borrowed the one line answer provided by Mottie and inserted it into diffViewerPageView.js, and as far as I can tell it solved the problem in both Firefox 35.0.1 on Windows and 31.3.0 ESR on Linux.  It also doesn't seem to break anything in IE 11.0.9600.

I know absolutely nothing about javascript so I don't really understand why this solution works or why it would be preferable to another solution.  Given this, I feel more comfortable posting the diff here and will leave it to someone else to publish for review.

diff --git a/reviewboard/static/rb/js/pages/views/diffViewerPageView.js b/reviewboard/static/rb/js/pages/views/diffViewerPageView.js
index 5a6557f..b815510 100644
--- a/reviewboard/static/rb/js/pages/views/diffViewerPageView.js
+++ b/reviewboard/static/rb/js/pages/views/diffViewerPageView.js
@@ -79,20 +79,21 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({
         /*
          * If we have "index_header" or a file+line hash in the location,
          * strip it off. Backbone's router makes use of the hash to try to
          * be backwards compatible with browsers that don't support the
          * history API, but we don't care about those, and if it's present
          * when we call start(), it will change the page's URL to be
          * /diff/index_header, which isn't a valid URL.
          */
         if (window.location.hash) {
             window.location.replace('#');
+            $('link[type*=icon]').detach().appendTo('head');
         }
 
         Backbone.history.start({
             pushState: true,
             hashChange: false,
             root: this.options.reviewRequestData.reviewURL + 'diff/',
             silent: true
         });
     },

As far as I can tell this was fixed in 42aee3b.  I now correctly see the favicon on diff viewer pages.  Confirmed in Firefox 36.0.1 on Windows and 31.3.0 ESR on Linux.

Neat.

• Status:

  -	New
  +	Fixed