Closed
Bug 1119560
Opened 9 years ago
Closed 9 years ago
remove unused browser.search.param.yahoo-fr* prefs
Categories
(Firefox :: Search, defect)
Firefox
Search
Tracking
()
RESOLVED
FIXED
Firefox 38
People
(Reporter: Gavin, Assigned: abdelrahman, Mentored)
References
Details
(Whiteboard: [good first bug])
Attachments
(1 file, 2 obsolete files)
5.32 KB,
patch
|
Details | Diff | Splinter Review |
See bug 995390 comment 6. These are unused now and can be removed from all of our default pref files: https://mxr.mozilla.org/mozilla-central/search?string=yahoo-fr
Reporter | ||
Comment 1•9 years ago
|
||
The reference in yahoometrofx.xml can be ignored since that plugin is no longer used.
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8546609 -
Flags: review?(gavin.sharp)
Reporter | ||
Comment 3•9 years ago
|
||
Comment on attachment 8546609 [details] [diff] [review] rev 1 - Remove unused prefs Please omit the change to the yahoometrofx.xml file, that file is also unused (and will be removed). r=me with that change reverted. The "ms-pc-metro" pref you're removing from browser/branding/official/pref/firefox-branding.js is not actually in scope for this bug, but since it is only used by a metro search plugin (which will soon be removed), that's fine. Thanks for the patch!
Attachment #8546609 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 4•9 years ago
|
||
Attachment #8546929 -
Flags: review?(gavin.sharp)
Reporter | ||
Comment 5•9 years ago
|
||
Comment on attachment 8546929 [details] [diff] [review] rev 2 - Remove unused prefs First, apologies for the delayed review response. My mention of "ms-pc-metro" in comment 3 was not meant to suggest that you should undo that change, I was just providing some context for posterity. "That's fine" meant that you should make that change, so please add that back in. In general, if you receive r+ with instruction to make a change, you do not need to request review again on the updated patch, as long as you've only made the requested change. If you attach an updated patch here, you can also go ahead and add r=gavin to the commit message, so that it can easily be checked in. This looks good with all of that taken into account, thanks again.
Attachment #8546929 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 6•9 years ago
|
||
Can rev 2 be Obsoleted? and just check-in rev 1.
Assignee | ||
Comment 7•9 years ago
|
||
(In reply to Abdelrhman Ahmed from comment #6) > Can rev 2 be Obsoleted? and just check-in rev 1. and also do I have privilege to perform check-in?
Flags: needinfo?(gavin.sharp)
Reporter | ||
Comment 8•9 years ago
|
||
Rev 1 is not suitable because it contains the yahoometrofx.xml change (which must be excluded). Rev 2 is not suitable because it does not remove browser.search.param.ms-pc-metro from browser/branding/nightly/pref/firefox-branding.js. Rev 3 should include the removal of browser.search.param.ms-pc-metro from browser/branding/nightly/pref/firefox-branding.js, but not change yahoometrofx.xml. It would be best to attach a new patch so that you can also include the revised commit comment (including "r=gavin").
Flags: needinfo?(gavin.sharp)
Assignee | ||
Comment 9•9 years ago
|
||
Attachment #8546609 -
Attachment is obsolete: true
Attachment #8546929 -
Attachment is obsolete: true
Reporter | ||
Comment 10•9 years ago
|
||
Thanks, now I'll mark this checkin-needed so that someone can check it in for you. (No try push needed since this is a simple pref removal.)
Assignee: nobody → a.ahmed1026
Keywords: checkin-needed
Reporter | ||
Updated•9 years ago
|
Mentor: gavin.sharp
Comment 11•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/c8bca1420a46
Keywords: checkin-needed
Whiteboard: [good first bug] → [good first bug][fixed-in-fx-team]
Comment 12•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c8bca1420a46
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][fixed-in-fx-team] → [good first bug]
Target Milestone: --- → Firefox 38
Comment 13•9 years ago
|
||
tl;dr this bug removed codes that are used outside of mozilla-central, namely localized files. We should be careful at all times with anything related to partner codes.
Comment 14•9 years ago
|
||
i presume mike's comment 13 relates to bug 1195683
You need to log in
before you can comment on or make changes to this bug.
Description
•