Remove unused lightweight theme footers

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Toolbars and Customization
RESOLVED FIXED
7 months ago
6 months ago

People

(Reporter: catlee, Assigned: catlee)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

59 bytes, text/x-review-board-request
Gijs
: review+
Details | Review
(Assignee)

Description

7 months ago
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
(Assignee)

Comment 1

7 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fd11030b7ae3
Comment hidden (mozreview-request)

Comment 3

7 months ago
Did you intend to request review?
Flags: needinfo?(catlee)
(Assignee)

Comment 4

7 months ago
(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)

Comment 5

7 months ago
(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)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

7 months ago
Attachment #8869549 - Attachment is obsolete: true
(Assignee)

Updated

7 months ago
Attachment #8869151 - Flags: review?(gijskruitbosch+bugs)

Comment 9

7 months ago
mozreview-review
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+

Updated

7 months ago
Assignee: nobody → catlee
Status: NEW → ASSIGNED
(Assignee)

Comment 10

7 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=007a7c612608
Comment hidden (mozreview-request)

Comment 12

7 months ago
Pushed by catlee@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/26681896ae7b
Remove LWT footers r=Gijs

Comment 13

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/26681896ae7b
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
(Assignee)

Comment 14

6 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=395a08e419e2
You need to log in before you can comment on or make changes to this bug.