Closed Bug 1119560 Opened 9 years ago Closed 9 years ago

remove unused browser.search.param.yahoo-fr* prefs

Categories

(Firefox :: Search, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 38

People

(Reporter: Gavin, Assigned: abdelrahman, Mentored)

References

Details

(Whiteboard: [good first bug])

Attachments

(1 file, 2 obsolete files)

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
The reference in yahoometrofx.xml can be ignored since that plugin is no longer used.
Attached patch rev 1 - Remove unused prefs (obsolete) — Splinter Review
Attachment #8546609 - Flags: review?(gavin.sharp)
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+
Attached patch rev 2 - Remove unused prefs (obsolete) — Splinter Review
Attachment #8546929 - Flags: review?(gavin.sharp)
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+
Can rev 2 be Obsoleted? and just check-in rev 1.
(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)
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)
Attachment #8546609 - Attachment is obsolete: true
Attachment #8546929 - Attachment is obsolete: true
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
Mentor: gavin.sharp
https://hg.mozilla.org/integration/fx-team/rev/c8bca1420a46
Keywords: checkin-needed
Whiteboard: [good first bug] → [good first bug][fixed-in-fx-team]
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
Blocks: 1195683
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.
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.