1335409 - Unused chrome://browser/skin/sync-128.png file

Status: RESOLVED FIXED

(Firefox :: Sync, defect, P3)

Description

[Florian Quèze [:florian]]

Bug 1296767 removed browser/base/content/sync/customize.css which contained the only referenced to chrome://browser/skin/sync-128.png at https://hg.mozilla.org/integration/autoland/rev/a30baab71ce2#l6.15

Comment 1

[indraniltiwary]

Hey Florian,
 I would like to work on this bug. If anyone can guide and mentor me on how to do this.
This would be my first bug.

Comment 2

[Florian Quèze [:florian]]

(In reply to indraniltiwary from comment #1)
> Hey Florian,
>  I would like to work on this bug. If anyone can guide and mentor me on how
> to do this.
> This would be my first bug.

Hi. Mark indicated Edouard will be mentoring this bug, I'll let him guide you.

Comment 3

[Edouard Oger [:eoger]]

Hi Indranil,

This bug is a fairly simple one, you just have to be thorough :)
Here's the work to do:

- There are 3 different sync-128.png images, one per platform (osx, linux, windows) located in browser/themes/[platform]/sync-128.png. Delete them.
- We reference these images in our build system using JAR Manifests (completely optional read - https://gecko.readthedocs.io/en/latest/build/buildsystem/jar-manifests.html), remove these references in browser/themes/[platform]/jar.mn

Once you're done, rebuild your browser and take it for a spin, just to make sure everything's OK (it should be, because these files are not used anywhere now).

Good luck!

Comment 4

[kevin.kwong.chip]

Hi Edouard, i'd like to also try this bug out as my good first bug. If I built mozilla following these instructions found here: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build/Linux_and_MacOS_build_preparation
would that be the correct branch version to fix this bug on?

Comment 5

[Edouard Oger [:eoger]]

Hi Kevin, I think you got beat by Indranil. Let's give him a week to work on this bug, if he doesn't the bug is all yours.

Comment 6

[kevin.kwong.chip]

Attachment: proposed patch

hey Edouard, I've made changes locally and produced a diff file. Please let me know if/how to take the patch further, thanks.

Comment 7

[Edouard Oger [:eoger]]

Comment on attachment 8838814 [details] [diff] [review]
proposed patch

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

Hello Kevin,

Thank you for your patch!
Now that you have removed the references to these sync-128.png images, you have to remove the files themselves.
As a reminder, they are located in browser/themes/[platform]/sync-128.png.

Please amend your patch with theses changes and change the commit message to something like:
Bug 1335409 - Remove unused sync-128.png image. r=eoger

Comment 8

[kevin.kwong.chip]

Attachment: kevin-proposed-change.patch

Thanks for getting back Edouard, I've made your requested changes and amended them to the previous patch file.

Comment 9

[Edouard Oger [:eoger]]

Comment on attachment 8839160 [details] [diff] [review]
kevin-proposed-change.patch

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

I think you attached the wrong patch, this one is for bug 1331166 :)

Comment 10

[kevin.kwong.chip]

Attachment: revised-change.patch

really sorry about that, here it is.

Comment 11

[Edouard Oger [:eoger]]

Comment on attachment 8839631 [details] [diff] [review]
revised-change.patch

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

r+! Thank you Kevin. I'll ask for a sheriff to land your patch on your behalf.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dec25fc08dcf
Remove unused sync-128.png image. r=eoger

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/dec25fc08dcf