Add favicon to the Add-ons SDK API docs

RESOLVED FIXED in 1.0

Status

Add-on SDK
Documentation
P3
normal
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: dbuc, Assigned: wbamberg)

Tracking

unspecified

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
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
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".
Component: Jetpack SDK → General
Product: Mozilla Labs → Add-on SDK
QA Contact: jetpack-sdk → general
(Assignee)

Updated

7 years ago
Assignee: nobody → wbamberg
(Reporter)

Comment 2

7 years ago
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
(Assignee)

Updated

7 years ago
Assignee: wbamberg → nobody
Component: General → Documentation
QA Contact: general → documentation
Priority: -- → P3
Target Milestone: --- → 1.0
(Assignee)

Updated

7 years ago
Assignee: nobody → wbamberg
(Assignee)

Comment 3

7 years ago
Fixed by: https://github.com/mozilla/addon-sdk/commit/3aa49d0c38c3514576d6bbc5d736b5469667ffa9
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Reporter)

Comment 4

7 years ago
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!)
(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.
(Assignee)

Updated

7 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 6

7 years ago
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.
(Assignee)

Comment 7

7 years ago
(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.
(Reporter)

Comment 8

7 years ago
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
(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.
(Assignee)

Comment 10

7 years ago
Created attachment 523698 [details] [diff] [review]
Change favico extension and link attribute
Attachment #523698 - Flags: review?
(Assignee)

Updated

7 years ago
Attachment #523698 - Flags: review? → review?(dbuchner)
(Assignee)

Comment 11

7 years ago
(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?
(Assignee)

Updated

7 years ago
Attachment #523698 - Flags: review?(dbuchner) → review?
(Assignee)

Comment 12

7 years ago
Created attachment 523721 [details] [diff] [review]
New, hopefully non-broken, patch

Sorry, that last patch was broken.
Attachment #523698 - Attachment is obsolete: true
Attachment #523721 - Flags: review?(dbuchner)
Attachment #523698 - Flags: review?
(Reporter)

Comment 13

7 years ago
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 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.
Attachment #523721 - Flags: review?(dbuchner) → review+
(Assignee)

Comment 15

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

I'm leaving this open for QA verification...
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.
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.