801044 - Update /favicon.ico to the new "M"

Status: RESOLVED FIXED

(www.mozilla.org :: Pages & Content, defect)

Description

[Isac Lagerblad (:icaaq)]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:19.0) Gecko/19.0 Firefox/19.0
Build ID: 20121011030552

Steps to reproduce:

1. Press Cmd+N to open a new browser window, then type https://www.mozilla.org/favicon.ico in the address bar and press Enter.


Actual results:

The firefox icon is displayed


Expected results:

the new mozilla icon should be displayed

Comment 1

[Anthony Ricaud (:rik)]

Steven: Can you take a look? I believe that's hitting the PHP codebase.

Comment 2

[Steven Garrity [:sgarrity]]

(In reply to Anthony Ricaud (:rik) from comment #1)
> Steven: Can you take a look? I believe that's hitting the PHP codebase.

You're right. It's probably worth having a copy of the new favicon at /favicon.ico, as some browsers may ignore the <link rel="shortcut icon"> location and just look for favicon.ico in the root.

We could:

1. Use a symlink to point the favicon.ico to /media/img/favicon.ico on the server side, or a redirect? Or;

2. Move the favicon from /media/img/favicon.ico to the root, and update the link/rel to point to that.

I'm not sure how to host a resource in the route with bedrock.

Thoughts?

Comment 3

[Isac Lagerblad (:icaaq)]

Just for the record, I noticed this when I view-sourced a page in google chrome

view-source:https://www.mozilla.org/en-US/

Comment 4

[Paul [:pmac] McLanahan]

It should be an easy update, it's just sitting in the root of the mozilla.com svn repo:

http://viewvc.svn.mozilla.org/vc/projects/mozilla.com/trunk/favicon.ico?view=log

Could also do a rewrite rule to the one in bedrock, and that might be the longer term solution for when (WHEN!) the PHP side goes away.

Comment 5

[Paul [:pmac] McLanahan]

I went ahead and updated the file in svn (trunk) in rev 110231.

We should however still considering doing this in apache for the long term.

Comment 6

[Kohei Yoshino [:kohei]]

Attachment: patch for header.inc.php

I just noticed this had not been merged to production yet. I can still see the old Firefox favicon https://www.mozilla.org/favicon.ico on PHP pages.

This will resolve Bug 683153, but all PHP pages including the Aurora/Beta landing pages and Firefox release notes will use this favicon. We may need a little tweak in header.inc.php as attached. This patch uses the same favicon and og:image as Bedrock, while https://www.mozilla.org/favicon.ico is still used on some pages that are not using the default header.

Comment 7

[Kohei Yoshino [:kohei]]

To be clear, we need both r110231 and this patch.

Comment 8

[Paul [:pmac] McLanahan]

Comment on attachment 828316 [details] [diff] [review]
patch for header.inc.php

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

Just a couple questions on /firefox/ url detection in the PHP.

::: includes/header.inc.php
@@ +41,4 @@
>  $fonts        = empty($fonts)        ? $default_fonts        : $fonts;
>  $styles       = empty($styles)       ? $default_styles       : $styles;
>  
> +if (strpos($_SERVER['REDIRECT_URL'], '/firefox/') === 0) {

why $_SERVER['REDIRECT_URL']? Wouldn't $_SERVER['PATH_INFO'] or $_SERVER['REQUEST_URI'] make more sense? Also /firefox/ might not be at position 0, there could be a locale in front of it.

Comment 9

[Kohei Yoshino [:kohei]]

(In reply to Paul McLanahan [:pmac] from comment #8)
> why $_SERVER['REDIRECT_URL']? Wouldn't $_SERVER['PATH_INFO'] or
> $_SERVER['REQUEST_URI'] make more sense? Also /firefox/ might not be at
> position 0, there could be a locale in front of it.

The REQUEST_URI has a locale, but the REDIRECT_URL doesn't. That's why.

Comment 10

[Paul [:pmac] McLanahan]

Yeah, but it doesn't seem REDIRECT_URL is guaranteed to exist.

http://php.net/manual/en/reserved.variables.server.php

Perhaps using REQUEST_URI like so would be more robust:

if (strpos($_SERVER['REQUEST_URI'], '/firefox/') !== false)

Comment 11

[Paul [:pmac] McLanahan]

New favicon merged to prod in r122532.

Comment 12

[Kohei Yoshino [:kohei]]

REDIRECT_URL is used in other PHP scripts so it should be safe, but REQUEST_URI works well. Tested locally and committed to trunk at r122565.

Comment 13

[Kohei Yoshino [:kohei]]

Actually r122565 is not merged yet, but it's okay, the legacy PHP site will be gone.

Comment 14

[Isac Lagerblad (:icaaq)]

Ahh, i looked at https://www.mozilla.org/favicon.ico and saw a "M" :)