1208121 - Remove some unused images in the browser theme

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Tim Nguyen :ntim]

browser/themes/shared/magnifier.png
browser/themes/shared/[e-mail]
browser/themes/windows/tabview/tabview-inverted.png

Comment 1

[Tim Nguyen :ntim]

To fix this bug, you need to remove the files mentioned above, and remove their reference in browser/themes/shared/jar.inc.mn and browser/themes/windows/jar.mn.

Comment 2

[Shaif]

So, I am working on this bug. could someone please assign me??

Comment 3

[Shaif]

Attachment: bug1208121.diff

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]:
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: 
[String/UUID change made/needed]:

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8670930 [details] [diff] [review]
bug1208121.diff

Hi Shaif, the patch that you attached is blank. You might want to join #introduction on the Mozilla IRC server to get help in creating the patch. (Or maybe you just need to run `hg qrefresh` if you're using Mercurial queues)

Comment 5

[Shaif]

Attachment: removed unused images from theme

Had attached diff file by mistake last time.
Please review the updated patch.

Comment 6

[Tim Nguyen :ntim]

Comment on attachment 8671997 [details] [diff] [review]
removed unused images from theme

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

Thanks for the patch ! I've added a few comments. Also, can you add a commit message and commit info for this patch ? See https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#Commit_Message_Conventions

::: browser/themes/shared/jar.inc.mn
@@ +65,5 @@
>    skin/classic/browser/identity-mixed-active-loaded.svg        (../shared/identity-block/identity-mixed-active-loaded.svg)
>    skin/classic/browser/info.svg                                (../shared/info.svg)
>    skin/classic/browser/tracking-protection-16.svg              (../shared/identity-block/tracking-protection-16.svg)
>    skin/classic/browser/tracking-protection-disabled-16.svg     (../shared/identity-block/tracking-protection-disabled-16.svg)
> +           

nit: Please remove this blank line.

::: browser/themes/windows/jar.mn
@@ +251,5 @@
>    skin/classic/browser/tabview/grain.png                      (tabview/grain.png)
>    skin/classic/browser/tabview/search.png                     (tabview/search.png)
>    skin/classic/browser/tabview/stack-expander.png             (tabview/stack-expander.png)
>    skin/classic/browser/tabview/tabview.png                    (tabview/tabview.png)
> +       skin/classic/browser/tabview/tabview.css                    (tabview/tabview.css)

Please restore the indentation on this line to 2 spaces (as it was before).

Comment 7

[Tim Nguyen :ntim]

Comment on attachment 8671997 [details] [diff] [review]
removed unused images from theme

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

Also, it seems you only removed the references to the files, and forgot to remove the files themselves from the source code.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Tim, you can handle the review for this patch. I am fine with this patch getting r=ntim when Tim believes the patch is sufficient.

Comment 9

[Shaif]

Attachment: bug1208121.diff

Comment 10

[Tim Nguyen :ntim]

Comment on attachment 8672206 [details] [diff] [review]
bug1208121.diff

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

Looks good to me ! Can you make sure your patch has commit info as mentioned in comment #6 ? It's needed for your patch to be checked in. If you have trouble adding commit info to your patch, feel free to ask in #introduction on IRC.


(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #8)
> Tim, you can handle the review for this patch. I am fine with this patch
> getting r=ntim when Tim believes the patch is sufficient.

Thanks !

Comment 11

[Shaif]

Attachment: bug1208121.diff

Added commit messege..

Comment 12

[Tim Nguyen :ntim]

Comment on attachment 8672265 [details] [diff] [review]
bug1208121.diff

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

The commit message should normally be formatted like this as the doc mentions:
Bug 1208121 - Remove some unused images in the browser theme. r=ntim

Also, this patch still misses commit user info (so we can identify who made the patch), there should be a line between `# HG changeset patch` and `# Parent (...)` that looks like this: 
# Your Name <[e-mail]>

Comment 13

[Shaif]

Attachment: bug1208121.diff

Comment 14

[Tim Nguyen :ntim]

Thanks for the patch ! I'm marking this for checkin, once the patch lands in mozilla-central, it'll appear on next day's Nightly.

Let me know if you'd like another bug to work on.

Comment 15

[Tim Nguyen :ntim]

Sheriffs, please see comment #8 for my permission to review this patch.

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/3eea09a56dc9

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3eea09a56dc9