Closed
Bug 1471305
Opened 6 years ago
Closed 6 years ago
Remove obsolete footerURL code
Categories
(WebExtensions :: Themes, enhancement, P3)
WebExtensions
Themes
Tracking
(firefox64 verified)
VERIFIED
FIXED
mozilla64
Tracking | Status | |
---|---|---|
firefox64 | --- | verified |
People
(Reporter: ntim, Assigned: championshuttler, Mentored)
References
(Blocks 1 open bug)
Details
(Keywords: good-first-bug)
Attachments
(1 file)
footerURL is not displayed anywhere in Firefox, let's remove that.
Comment 1•6 years ago
|
||
Please tell more about this.
How can I work on this?
Flags: needinfo?(ntim.bugs)
Reporter | ||
Comment 2•6 years ago
|
||
Here's the code you'll need to remove:
https://dxr.mozilla.org/mozilla-central/search?q=footerURL&case=false®exp=false&path=toolkit
Flags: needinfo?(ntim.bugs)
Updated•6 years ago
|
Assignee: nobody → 1991manish.kumar
Updated•6 years ago
|
Priority: -- → P3
Comment 3•6 years ago
|
||
what about these kinds of lines?
https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/LightweightThemeManager.jsm#26
What to do with 'footerURL' here and how can I test?
Flags: needinfo?(ntim.bugs)
Reporter | ||
Comment 4•6 years ago
|
||
In that specific case, you should remove `"footerURL", ` from the array.
> how can I test?
Just make sure switching themes in the customize mode still works (Menu > Customize > Themes).
Flags: needinfo?(ntim.bugs)
Updated•6 years ago
|
Assignee: 1991manish.kumar → nobody
Reporter | ||
Updated•6 years ago
|
Mentor: ntim.bugs
Keywords: good-first-bug
Assignee | ||
Comment 5•6 years ago
|
||
Hi, i would like to work on this issue,
Reporter | ||
Comment 6•6 years ago
|
||
(In reply to Shivam Singhal [:championshuttler] from comment #5)
> Hi, i would like to work on this issue,
In case you haven't, please set up a development environment (see https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction ). Once that's done, feel free to remove the code from comment 2, and attach a patch once you're done.
Please let me know if you need help.
Updated•6 years ago
|
Assignee: nobody → shivams2799
Assignee | ||
Comment 7•6 years ago
|
||
Removing footerURL code , few of the lines are left , i am not sure how to delete them and what to use instead of them there.
Reporter | ||
Comment 8•6 years ago
|
||
Comment on attachment 9008838 [details]
Bug 1471305 - Remove obsolete footerURL code.
Tim Nguyen :ntim has approved the revision.
Attachment #9008838 -
Flags: review+
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e60665194092
Remove obsolete footerURL code. r=ntim
Comment 10•6 years ago
|
||
Backout by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9bb04fb52dd7
Backed out changeset e60665194092 for eslint failure at extensions/internal/LightweightThemeImageOptimizer.jsm on a CLOSED TREE
Assignee | ||
Comment 11•6 years ago
|
||
Hi ntim , can you tell me what happened, did the commit revert now?
Flags: needinfo?(ntim.bugs)
Reporter | ||
Comment 12•6 years ago
|
||
(In reply to Shivam Singhal [:championshuttler] from comment #11)
> Hi ntim , can you tell me what happened, did the commit revert now?
Yes, the commit was reverted. There was a ESLint failure: https://treeherder.mozilla.org/logviewer.html#?job_id=199355439&repo=autoland&lineNumber=66
Your patch made ORIGIN_BOTTOM_LEFT unused, so you'll need to remove that variable definition: https://searchfox.org/mozilla-central/rev/99cbc0aec3e1c0b65ff9052523fb5c181b248f57/toolkit/mozapps/extensions/internal/LightweightThemeImageOptimizer.jsm#18
Once you've done that, the patch should be good to go.
Flags: needinfo?(ntim.bugs)
Comment 13•6 years ago
|
||
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/abcfb507d1e9
Remove obsolete footerURL code. r=ntim
Comment 14•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Comment 15•6 years ago
|
||
Verified according to comment #4 using Nightly 64.0a1 (2018-09-16) running on Windows 10 x64, no issues were found with the themes on the new version.
Updated•6 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•