Closed Bug 1288910 Opened 3 years ago Closed 3 years ago

update Firefox_Installer_Title.png & Flash_Icon_130x130.png in b2g-installer

Categories

(Firefox OS Graveyard :: B2gInstaller, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: kskarthik91, Assigned: kskarthik91)

References

()

Details

Attachments

(3 files, 2 obsolete files)

Firefox_Installer_Title.png needs to be renamed
Please review this bug gerard-majax
Mentor: lissyx+mozillians
Status: UNCONFIRMED → RESOLVED
Closed: 3 years ago
Flags: a11y-review+
Resolution: --- → INVALID
why are you marking this as invalid? I suggest you add your patch and flag Alexandre for review, so he sees it on monday...
Flags: needinfo?(kskarthik91)
[Tracking Requested - why for this release]:
Status: RESOLVED → UNCONFIRMED
Flags: needinfo?(lissyx+mozillians)
Flags: needinfo?(kskarthik91)
Flags: a11y-review-
Flags: a11y-review+
Resolution: INVALID → ---
Attached file kskarthik:master > mozilla-b2g:master (obsolete) —
Attachment #8774108 - Flags: review?(lissyx+mozillians)
Flags: a11y-review-
Assignee: nobody → kskarthik91
Flags: needinfo?(lissyx+mozillians)
Comment on attachment 8774108 [details] [review]
kskarthik:master > mozilla-b2g:master

Please make sure you are using the same font. That looks like it is not the case.
Also, rename the file for consistency :)
Flags: needinfo?(kskarthik91)
Attachment #8774108 - Flags: review?(lissyx+mozillians)
Hey Amy, do you remember the name of the font used for the b2g installer title ? Thanks!
Flags: needinfo?(amlee)
(In reply to Alexandre LISSY :gerard-majax from comment #6)
> Hey Amy, do you remember the name of the font used for the b2g installer
> title ? Thanks!

Hi, 

The font is Helvetica Neue DeskInterface Thin

Visual spec can be found here (pg 11): 
https://bug1166276.bmoattachments.org/attachment.cgi?id=8641894

Cheers, 
Amy
Flags: needinfo?(amlee)
(In reply to Amy Lee [:amylee] UX from comment #7)
> (In reply to Alexandre LISSY :gerard-majax from comment #6)
> > Hey Amy, do you remember the name of the font used for the b2g installer
> > title ? Thanks!
> 
> Hi, 
> 
> The font is Helvetica Neue DeskInterface Thin
> 
> Visual spec can be found here (pg 11): 
> https://bug1166276.bmoattachments.org/attachment.cgi?id=8641894
> 
> Cheers, 
> Amy

https://github.com/mozilla-b2g/b2g-installer/pull/50/commits/ac1b05ffe53ceab14ef290f576fabecd2c908d6d plsease see this font. I think both are same
Flags: needinfo?(kskarthik91) → needinfo?(lissyx+mozillians)
Can you please:
 - make sure the title font size is the same as the original file
 - squash all your commits into one on the PR ?

Once this is done, please flag the attachment for review :)
Flags: needinfo?(lissyx+mozillians) → needinfo?(kskarthik91)
Attachment #8776900 - Flags: review?(lissyx+mozillians)
Assignee: kskarthik91 → lissyx+mozillians
Mentor: lissyx+mozillians
Comment on attachment 8776900 [details] [review]
b2g installer patch for title image & icon

This is still not addressing what I said earlier about font.
Attachment #8776900 - Flags: review?(lissyx+mozillians)
And why do you create so many pull requests ?
(In reply to Alexandre LISSY :gerard-majax from comment #12)
> And why do you create so many pull requests ?

i fixed the font as per mozilla style
(In reply to kskarthik from comment #13)
> (In reply to Alexandre LISSY :gerard-majax from comment #12)
> > And why do you create so many pull requests ?
> 
> i fixed the font as per mozilla style

There is no "Bug XX - yyy r=zz" on the commit itself.
Font size is not the same between both titles, and spacing is also different.
Assignee: lissyx+mozillians → kskarthik91
it's hard to reach your expectations...
Size changed as the letters are decreased. Spacing won't cause an issue.
Flags: needinfo?(kskarthik91)
i will merge the commits to one. Please pull. If you aren't satisfied with it, you can undo it :)
i will merge the commits to one. Please pull. If you aren't satisfied with it, you can undo it :)
Summary: Rename Firefox_Installer_Title.png in B2G Installer → update Firefox_Installer_Title.png & Flash_Icon_130x130.png
Summary: update Firefox_Installer_Title.png & Flash_Icon_130x130.png → update Firefox_Installer_Title.png & Flash_Icon_130x130.png in b2g-installer
Comment on attachment 8774108 [details] [review]
kskarthik:master > mozilla-b2g:master

>Error retrieving Github pull request diff for https://github.com/mozilla-b2g/b2g-installer/pull/52
Attachment #8774108 - Flags: review?(lissyx+mozillians)
Please update the attachments, we have several of them pointing on closed PR and the proper PR is just in a comment, it's hard to track :(
Flags: needinfo?(kskarthik91)
Attached file B2G Installer PR
Attachment #8774108 - Attachment is obsolete: true
Attachment #8776900 - Attachment is obsolete: true
Attachment #8774108 - Flags: review?(lissyx+mozillians)
Attachment #8791900 - Flags: review?(lissyx+mozillians)
I am sorry but this is still not fixing the font you are using. Please request review once you fix this :)
Comment on attachment 8791900 [details] [review]
B2G Installer PR

Font needs to be consistent accross everything.
Attachment #8791900 - Flags: review?(lissyx+mozillians)
(In reply to Alexandre LISSY :gerard-majax from comment #23)
> Comment on attachment 8791900 [details] [review]
> B2G Installer PR
> 
> Font needs to be consistent accross everything.

Alexandre, I was trying to help on this. If it's just an issue related to font dimension, can you help me in understanding which is the original font size? Thank you for your help. Luca
(In reply to lucaperri87 from comment #24)
> (In reply to Alexandre LISSY :gerard-majax from comment #23)
> > Comment on attachment 8791900 [details] [review]
> > B2G Installer PR
> > 
> > Font needs to be consistent accross everything.
> 
> Alexandre, I was trying to help on this. If it's just an issue related to
> font dimension, can you help me in understanding which is the original font
> size? Thank you for your help. Luca

Sure, it is already documented in comment 7 above :)
Flags: needinfo?(lucaperri87)
Uhm, so the font is not a "Fira Sans" one, but OS X - Helvetica Neue DeskInterface Thin, 55px.

At least, we now know the correct one :) Unfortunately I'm not a Mac user, so I can't play with this font.

May we switch (obviously we can convert all the elements needed) to an open font? or is there somewhere I can grab it legally?
Technically I just care that we do not mix different font. I just learnt that this was not a freely available font, so I don't mind that we use another: just make sure we use it *everywhere*.

Adding a screenshot to the PR review might help :)
Attachment #8792137 - Flags: review?(lissyx+mozillians)
this is the result of my patch. I hope it is to every ones satisfaction.
Looks good, thanks!

Sorry for the delay kskarthik, you should have told me the font was not available, I did not noticed :)
Flags: needinfo?(lucaperri87)
Flags: needinfo?(kskarthik91)
Attachment #8792137 - Flags: review?(lissyx+mozillians) → review+
https://github.com/mozilla-b2g/b2g-installer/commit/546af4c2a7c27bd43112b6a3851873c6da2dbe00
Status: UNCONFIRMED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Thanks all of you!
Finally! Thank you very much guys for all your support :)
You need to log in before you can comment on or make changes to this bug.