Remove browser/themes/windows/loop

RESOLVED FIXED in Firefox 55

Status

()

P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: florian, Assigned: ganesh2583, Mentored)

Tracking

({good-first-bug})

Trunk
Firefox 55
Unspecified
Windows
good-first-bug
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
These 2 files don't seem to be used anywhere:

browser/themes/windows/loop/toolbar-win10.png
browser/themes/windows/loop/toolbar-win10@2x.png

And when looking at https://hg.mozilla.org/mozilla-central/rev/f579b4fc5bf5, it seems these files have never actually been packaged, and the files we actually used for Hello were at:

browser/extensions/loop/skin/windows/toolbar-win10.png
browser/extensions/loop/skin/windows/toolbar-win10@2x.png
Mentor: dao+bmo
Keywords: good-first-bug
Priority: -- → P3
Whiteboard: [good first bug]
Version: 53 Branch → Trunk
(Assignee)

Comment 1

2 years ago
Hi Dao,

I would like to work on this bug. Please let me know how to proceed on this.

Regards
Ganesh
(In reply to ganesh2583 from comment #1)
> Hi Dao,
> 
> I would like to work on this bug. Please let me know how to proceed on this.
> 
> Regards
> Ganesh

Basically you just need to do 'hg rm browser/themes/windows/loop'
(Assignee)

Comment 3

2 years ago
Posted patch Patch for Bug 1343830 (obsolete) — Splinter Review
Hi,

I added the fix patch for this bug. Please review the changes and let me know if anything else is required.

Regards,
Ganesh
Attachment #8849572 - Flags: review?(dao+bmo)
(Assignee)

Comment 4

2 years ago
Hi Dao,

Can you also point me any other bugs where I can contribute more, from coding and application understanding perspective. My skill set include JavaScript, Java and HTML/CSS. I would like to contribute more on that front.

Regards,
Ganesh
Comment on attachment 8849572 [details] [diff] [review]
Patch for Bug 1343830

This patch seems broken. It says "Binary file [...] has changed" (without listing any change) rather than "deleted file mode 100644".
Attachment #8849572 - Flags: review?(dao+bmo) → review-
(Assignee)

Comment 6

2 years ago
(In reply to Dão Gottwald [::dao] from comment #5)
> Comment on attachment 8849572 [details] [diff] [review]
> Patch for Bug 1343830
> 
> This patch seems broken. It says "Binary file [...] has changed" (without
> listing any change) rather than "deleted file mode 100644".

I think since the files inside the folder are .png files the patch shows are Binary Files. I did 'hg rm browser/themes/windows/loop' as well as from Hg Workbench I removed the folder, that is how the patch is getting generated in both the cases. Please let me know if you think otherwise.
Yes, it's in binary format, but the deletions aren't noted correctly. It doesn't just look wrong, I also tried to apply the patch locally and it didn't do anything :/
(Assignee)

Comment 8

2 years ago
I generate the previous patch from Hg Workbench. May be that was causing the issue. I not generate the patch using "hg export tip > "Bug 1343830.patch"" command. And the patch files seems to have the content you have mentioned. Please let me if any further changes are required.
Attachment #8849611 - Flags: review?(dao+bmo)
Comment on attachment 8849611 [details] [diff] [review]
Attached the correct patch now.

Looks good :)
Attachment #8849611 - Flags: review?(dao+bmo) → review+
Assignee: nobody → ganesh2583
Attachment #8849572 - Attachment is obsolete: true

Comment 10

2 years ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2e428a9ba3f
Remove browser/themes/windows/loop. r=dao
(Assignee)

Comment 11

2 years ago
(In reply to Dão Gottwald [::dao] from comment #9)
> Comment on attachment 8849611 [details] [diff] [review]
> Attached the correct patch now.
> 
> Looks good :)

Hey,

I think this message got lost in the disturbance, let me copy paste again :

Can you also point me any other bugs where I can contribute more, from coding and application understanding perspective. My skill set include JavaScript, Java and HTML/CSS. I would like to contribute more on that front.

:-)
(In reply to ganesh2583 from comment #11)
> Can you also point me any other bugs where I can contribute more, from
> coding and application understanding perspective. My skill set include
> JavaScript, Java and HTML/CSS. I would like to contribute more on that front.
> 
> :-)

Here's another one that I just filed: bug 1349502. It's not particularly complex but slightly more interesting than this one.
(Assignee)

Comment 13

2 years ago
(In reply to Dão Gottwald [::dao] from comment #12)
> (In reply to ganesh2583 from comment #11)
> > Can you also point me any other bugs where I can contribute more, from
> > coding and application understanding perspective. My skill set include
> > JavaScript, Java and HTML/CSS. I would like to contribute more on that front.
> > 
> > :-)
> 
> Here's another one that I just filed: bug 1349502. It's not particularly
> complex but slightly more interesting than this one.

Cool, let me comment on that bug to get started on it.

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a2e428a9ba3f
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.