Closed Bug 1363853 Opened 7 years ago Closed 7 years ago

Remove unused lightweight theme footers

Categories

(Firefox :: Toolbars and Customization, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 55
Tracking Status
firefox55 --- fixed

People

(Reporter: catlee, Assigned: catlee)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

59 bytes, text/x-review-board-request
Gijs
: review+
Details
We don't think we're using the theme footers in https://dxr.mozilla.org/mozilla-central/source/browser/base/content/defaultthemes any more, but we're still shipping them.

We should delete the footer images from hg, and any references to them.
https://dxr.mozilla.org/mozilla-central/search?q=regexp%3A%22%5B0-9%5D%5C.footer.(png%7Cjpg)%22&redirect=false
Did you intend to request review?
Flags: needinfo?(catlee)
(In reply to :Gijs from comment #3)
> Did you intend to request review?

Just looking for feedback right now, given that I don't understand this code at all. The builds and tests seem to run ok. Who is a good person to review this code?

I wasn't sure what to do with all the other references to 'footerURL' in the code:
https://dxr.mozilla.org/mozilla-central/search?q=footerURL&redirect=false
Flags: needinfo?(catlee) → needinfo?(gijskruitbosch+bugs)
(In reply to Chris AtLee [:catlee] from comment #4)
> (In reply to :Gijs from comment #3)
> > Did you intend to request review?
> 
> Just looking for feedback right now, given that I don't understand this code
> at all. The builds and tests seem to run ok. Who is a good person to review
> this code?

I could, or Dão or Mossop or Jared (:jaws).

> I wasn't sure what to do with all the other references to 'footerURL' in the
> code:
> https://dxr.mozilla.org/mozilla-central/search?q=footerURL&redirect=false

The toolkit ones might want to stay if Thunderbird uses them? Here's a narrower query that ignores that and the objdir hits:

https://dxr.mozilla.org/mozilla-central/search?q=footerURL+-path%3Atoolkit+-path%3Aobj-x&redirect=false

The mobile hit is harmless, so I guess you can leave it... but otherwise I think everything else should go.
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #8869549 - Attachment is obsolete: true
Attachment #8869151 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8869151 [details]
Bug 1363853: Remove LWT footers

https://reviewboard.mozilla.org/r/140782/#review145036

r=me, though I am a little confused about the toolkit test changes. I don't think those are strictly required. Was there a particular reason to alter those (ie do they all fail if you don't remove the footerURL stuff)? Inasmuch as we still use this stuff on other toolkit apps, perhaps the tests should stay... I don't feel super strongly, though, so it's up to you.
Attachment #8869151 - Flags: review?(gijskruitbosch+bugs) → review+
Assignee: nobody → catlee
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/26681896ae7b
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.