950536 - [ko] Rechange favicon of Naver and Daum's search plugin

Status: RESOLVED FIXED

(Mozilla Localizations :: ko / Korean, defect)

Description

[Channy Yun [:channy]]

Naver and Daum's favicon were changed recently.
http://www.naver.com/favicon.ico
http://www.daum.net/favicon.ico

Comment 1

[Channy Yun [:channy]]

Attachment: Change icons both Naver and Daum patch v1.0

Comment 2

[Channy Yun [:channy]]

Attachment: Change icons both Naver and Daum patch v1.1

Updated 32 pixel icons for mobile and 16 pixel for browser and mail

Comment 3

[Francesco Lodolo [:flod]]

Comment on attachment 8367875 [details] [diff] [review]
Change icons both Naver and Daum patch v1.0

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

I suggest to provide one patch for mail and another one for browser+mobile, since they need different reviewer. 
Ideally, one patch for product would be even easier to review.

Mobile: you need 32px icons. If you open the .ico files from those 2 websites, you'll find several sizes included, please use the 32px one (but keep height and width =16 in the xml file)

Comment 4

[Channy Yun [:channy]]

(In reply to Francesco Lodolo [:flod] from comment #3)
> Comment on attachment 8367875 [details] [diff] [review]
> Change icons both Naver and Daum patch v1.0
> 
> Review of attachment 8367875 [details] [diff] [review]:
> -----------------------------------------------------------------
> Mobile: you need 32px icons. If you open the .ico files from those 2
> websites, you'll find several sizes included, please use the 32px one (but
> keep height and width =16 in the xml file)

Can I use single 32 pixel icons for browser, mail and mobile with same base64 code?

Comment 5

[Francesco Lodolo [:flod]]

Unfortunately no.

16px icons for Mail and Firefox
32px with attributes set to 16px for Mobile

Comment 6

[Channy Yun [:channy]]

Attachment: Change icons both Naver and Daum patch v1.2 for browser and mobile

Comment 7

[Channy Yun [:channy]]

Attachment: Change icons both Naver and Daum patch v1.2 for mail

Comment 8

[Francesco Lodolo [:flod]]

Comment on attachment 8367907 [details] [diff] [review]
Change icons both Naver and Daum patch v1.2 for browser and mobile

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

Checked the images and they look good.

How exactly did you create the patch? The diff looks fine, but fails to apply to naver-kr.xml (both mobile and desktop) and I'm not sure why. My only guess is that something happened when you save the file.

At this point please go on and commit to aurora and l10n-central, reference both the bug and the review (e.g. "Bug 950536: [ko] Rechange favicon of Naver and Daum's search plugin, r=flod") in the commit message. When you're done, put a link to the changeset here so I can double check what changed.

Comment 9

[Channy Yun [:channy]]

fixed in http://hg.mozilla.org/releases/l10n/mozilla-aurora/ko/rev/8115d56d0ddd both browser and mobile

Comment 10

[Francesco Lodolo [:flod]]

Thanks, looks right. Can you land it on l10n-central too?

Comment 11

[Channy Yun [:channy]]

(In reply to Francesco Lodolo [:flod] from comment #10)
> Thanks, looks right. Can you land it on l10n-central too?

fixed in http://hg.mozilla.org/l10n-central/ko/rev/93e5824b2396 too.
I will do migration of full changed messages from aurora to central soon.

Thanks for your assistance. In case of beta repo, I guess somehow late to update them, right?

Comment 12

[Francesco Lodolo [:flod]]

Yes, Beta is definitely too late, no point in landing there since next Tuesday is merge day.

Please try to test the next build for mobile/desktop tomorrow and request a new sign-off, so that changes can move on to beta next week.

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 8367910 [details] [diff] [review]
Change icons both Naver and Daum patch v1.2 for mail

Sorry for the delay, looks good. r=Standard8