602915 - Add favicon to the Add-ons SDK API docs

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: Documentation, defect, P3)

Description

[Daniel Buchner [:dbuc]]

Now that app tabs are here I have been keeping the SDK docs up in an app tab 24/7/365, and its app tab shows a default blank page icon. This is becoming unwieldy as I have 2 other pages app tabbed that also show the default icon. As you can imagine a tab-centric version of a street hustler's shell game ensues... :)

Perhaps we can add this tried and true favicon to the page? https://jetpack.mozillalabs.com/images/jetpack.png

Comment 1

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".

Comment 2

[Daniel Buchner [:dbuc]]

Now that the docs are going to be accessible from AMO and the project has moved toward AMO in general, I think we should use this favicon instead (the AMO puzzle piece) - https://builder.addons.mozilla.org/media/base/img/favicon.ico

Comment 3

[Will Bamberg [:wbamberg]]

Fixed by: https://github.com/mozilla/addon-sdk/commit/3aa49d0c38c3514576d6bbc5d736b5469667ffa9

Comment 4

[Daniel Buchner [:dbuc]]

FYI, the logo is actually a PNG image not an ICO, our developers on Builder overlooked that and put it in the page with an ICO extension. It will still load the shortcut icon, but it gives this nag error in the console, change the extension to .PNG and you'll be gravy ;)  (we had to do the same!)

Comment 5

[Wil Clouser [:clouserw]]

(In reply to comment #4)
> FYI, the logo is actually a PNG image not an ICO, our developers on Builder
> overlooked that and put it in the page with an ICO extension. It will still
> load the shortcut icon, but it gives this nag error in the console, change the
> extension to .PNG and you'll be gravy ;)  (we had to do the same!)

Please have QA verify the bug then.  IE does weird things with favicons.

Comment 6

[Daniel Buchner [:dbuc]]

So two things, this is an app only intended for Firefox, and the PNG is fine. If you want confirmation of the bug, look at any page of the Builder in Firebug, it will throw this nag error: Image corrupt or truncated: <unknown>

The reason it does that, is the link tag is telling the parser that the image is a binary ICO but when it receives the image it parses it as the true file type that it is, PNG - but it throws that nag error to let you know the image has the wrong file extension.

Comment 7

[Will Bamberg [:wbamberg]]

(In reply to comment #6)
> So two things, this is an app only intended for Firefox, and the PNG is fine.
> If you want confirmation of the bug, look at any page of the Builder in
> Firebug, it will throw this nag error: Image corrupt or truncated: <unknown>
> 

But the docs are also viewable online (currently https://jetpack.mozillalabs.com/sdk/1.0b4/docs/) and that could use any browser. Or do I misunderstand you?

> The reason it does that, is the link tag is telling the parser that the image
> is a binary ICO but when it receives the image it parses it as the true file
> type that it is, PNG - but it throws that nag error to let you know the image
> has the wrong file extension.

Thanks for finding this Daniel.

As for QA, I'm not really sure what the process is for that, but perhaps I can ask Ayan Shah to look at it.

Comment 8

[Daniel Buchner [:dbuc]]

No additional QA is needed, we already did QA and fixed the issue as seen in this ticket: https://bugzilla.mozilla.org/show_bug.cgi?id=646664

Comment 9

[Wil Clouser [:clouserw]]

(In reply to comment #8)
> No additional QA is needed, we already did QA and fixed the issue as seen in
> this ticket: https://bugzilla.mozilla.org/show_bug.cgi?id=646664

That bug isn't marked verified by QA.  QA doesn't need to look at both, but they need to look at one, and in IE.

Comment 10

[Will Bamberg [:wbamberg]]

Attachment: Change favico extension and link attribute

Comment 11

[Will Bamberg [:wbamberg]]

(In reply to comment #9)
> (In reply to comment #8)
> > No additional QA is needed, we already did QA and fixed the issue as seen in
> > this ticket: https://bugzilla.mozilla.org/show_bug.cgi?id=646664
> 
> That bug isn't marked verified by QA.  QA doesn't need to look at both, but
> they need to look at one, and in IE.

Ayan: would you be able to QA-verify this fix?

Comment 12

[Will Bamberg [:wbamberg]]

Attachment: New, hopefully non-broken, patch

Sorry, that last patch was broken.

Comment 13

[Daniel Buchner [:dbuc]]

Cool that looks right, if I'm not mistaken we had to actually change the image (as in the actual asset on the server) as well because it was saved out there as an ICO. I'd just check it to be sure :)

Comment 14

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 523721 [details] [diff] [review]
New, hopefully non-broken, patch

(In reply to comment #13)
> Cool that looks right, if I'm not mistaken we had to actually change the image
> (as in the actual asset on the server) as well because it was saved out there
> as an ICO. I'd just check it to be sure :)

This patch does that, although the change doesn't show up in Bugzilla's diff view, which apparently can't handle the binary parts of git-style patches.

Comment 15

[Will Bamberg [:wbamberg]]

Fixed by: https://github.com/mozilla/addon-sdk/commit/1d65e57e409df2fa9be0ebfef11506ac90793107

I'm leaving this open for QA verification...

Comment 16

[Wil Clouser [:clouserw]]

Generally the flow is to close the bug which tells QA to look at it.  At that point they can either mark it REOPENED or VERIFIED.

Thanks.