Remove glyph-dropdown{,@2x}.png and replace usages with arrow-dropdown-12.svg

RESOLVED FIXED in Firefox 66

Status

()

enhancement
P3
normal
RESOLVED FIXED
7 months ago
4 months ago

People

(Reporter: ntim, Assigned: lauirvin98, Mentored)

Tracking

(Blocks 1 bug, {good-first-bug})

unspecified
mozilla66
Points:
---

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
Priority: -- → P3

Updated

4 months ago
Keywords: good-first-bug
(Assignee)

Comment 1

4 months ago
Hello, I'm new and I would like to work on this issue as my first bug.

Comment 2

4 months ago
Sure! submit the patch. 
Later someone (:ntim) will assign this to you.
If this is your first bug check this also:
https://wiki.mozilla.org/Good_first_bug

Either you can submit patch here or you can submit using phabricator.
(Reporter)

Comment 3

4 months ago
(In reply to Irvin Ives Lau from comment #1)
> Hello, I'm new and I would like to work on this issue as my first bug.

Sure! Once you've set up everything, you'll need to:

- Remove these two files:
toolkit/themes/osx/global/icons/glyph-dropdown.png
toolkit/themes/osx/global/icons/glyph-dropdown@2x.png

- Remove these lines:
https://searchfox.org/mozilla-central/rev/3160ddc1f0ab55d230c595366662c62950e5c785/browser/themes/osx/browser.css#790-799
https://searchfox.org/mozilla-central/rev/3160ddc1f0ab55d230c595366662c62950e5c785/browser/themes/osx/browser.css#833-842
https://searchfox.org/mozilla-central/rev/3160ddc1f0ab55d230c595366662c62950e5c785/toolkit/themes/osx/global/jar.mn#54-55

- And change these lines:

https://searchfox.org/mozilla-central/rev/3160ddc1f0ab55d230c595366662c62950e5c785/browser/themes/osx/browser.css#831
https://searchfox.org/mozilla-central/rev/3160ddc1f0ab55d230c595366662c62950e5c785/browser/themes/osx/browser.css#785

to use arrow-dropdown-12.svg instead of glyph-dropdown.png

Once you've done all those steps, you can submit a patch, then I'll assign the bug to you.

Comment 4

4 months ago
Irvin

If you need more information, you can ask in IRC #Introduction
or 
use "Need More Information from" feature here.

while submitting review also, tag someone, who can review your patch.
(Assignee)

Comment 5

4 months ago
Posted patch patch.diff (obsolete) — Splinter Review
Hi, thank you for assigning me the task, I have made a patch.

Please kindly review this and let me know if it is okay.
Attachment #9030790 - Flags: review?(ntim.bugs)
Attachment #9030790 - Flags: review?(1991manish.kumar)
(Reporter)

Comment 6

4 months ago
Comment on attachment 9030790 [details] [diff] [review]
patch.diff

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

Thanks for the patch! It looks like you haven't removed the files in this patch.

Can you do:

hg rm toolkit/themes/osx/global/icons/glyph-dropdown.png

hg rm toolkit/themes/osx/global/icons/glyph-dropdown@2x.png

from your terminal ?

::: browser/themes/osx/browser.css
@@ +782,4 @@
>  
>  button.translate-infobar-element.options-menu-button > .button-box > .button-menu-dropmarker {
>    display: -moz-box;
> +  list-style-image: url("chrome://../shared/icons/arrow-dropdown-12.svg");

This should be chrome://global/skin/icons/arrow-dropdown-12.svg

@@ +818,4 @@
>  
>  menulist.translate-infobar-element > .menulist-dropmarker {
>    display: -moz-box;
> +  list-style-image: url("chrome://../shared/icons/arrow-dropdown-12.svg");

Same here
Attachment #9030790 - Flags: review?(ntim.bugs)
Attachment #9030790 - Flags: review?(1991manish.kumar)
(Reporter)

Updated

4 months ago
Assignee: nobody → lauirvin98
(Assignee)

Comment 7

4 months ago
Posted patch patch.diffSplinter Review
Thank you for your review, I have now made the changes.

Please let me know if it's fine.
Attachment #9030809 - Flags: review?(ntim.bugs)
(Reporter)

Updated

4 months ago
Attachment #9030790 - Attachment is obsolete: true
(Reporter)

Comment 8

4 months ago
Comment on attachment 9030809 [details] [diff] [review]
patch.diff

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

Looks good to me!
Attachment #9030809 - Flags: review?(ntim.bugs)
Attachment #9030809 - Flags: review?(jaws)
Attachment #9030809 - Flags: feedback+
Attachment #9030809 - Flags: review?(jaws) → review+
Mentor: ntim.bugs
Status: NEW → ASSIGNED
Keywords: checkin-needed

Comment 9

4 months ago
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fcc5588a9a19
Remove glyph-dropdown{,@2x}.png and replace usages with arrow-dropdown-12.svg. r=jaws
Keywords: checkin-needed
(Reporter)

Comment 10

4 months ago
Thanks for working on this Irvin! Bug 1509634 is pretty similar if you're looking for more bugs :)
(Assignee)

Comment 11

4 months ago
Thank you for your guidance, I will definitely check it out!

(In reply to Tim Nguyen :ntim (please use needinfo?) from comment #10)
> Thanks for working on this Irvin! Bug 1509634 is pretty similar if you're
> looking for more bugs :)
(Assignee)

Updated

4 months ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → INVALID
(Reporter)

Updated

4 months ago
Status: RESOLVED → REOPENED
Resolution: INVALID → ---

Comment 12

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fcc5588a9a19
Status: REOPENED → RESOLVED
Last Resolved: 4 months ago4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.