Closed
Bug 1106150
Opened 9 years ago
Closed 9 years ago
condition="defaultEngine" MozParam broken by bug 1102416
Categories
(Firefox :: Search, defect)
Firefox
Search
Tracking
()
RESOLVED
FIXED
Firefox 37
People
(Reporter: Gavin, Assigned: Gavin)
References
Details
Attachments
(1 file)
954 bytes,
patch
|
markh
:
review+
Sylvestre
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
I forgot a spot where we check the defaultenginename in bug 1102416's patch. Thankfully, this only affects MozParams for the condition "defaultEngine", which we no longer use after bug 1103216.
Assignee | ||
Updated•9 years ago
|
status-firefox34:
--- → wontfix
status-firefox35:
--- → affected
status-firefox36:
--- → affected
tracking-firefox35:
--- → +
tracking-firefox36:
--- → +
Assignee | ||
Comment 1•9 years ago
|
||
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Comment 2•9 years ago
|
||
Want to uplift? If yes and this is safe/low risk, let's get it nommed and landed by Mon Dec 29th please.
Flags: needinfo?(gavin.sharp)
Assignee | ||
Comment 3•9 years ago
|
||
Comment on attachment 8530413 [details] [diff] [review] patch Mark: if you could help write a test for this, I'd appreciate it.
Flags: needinfo?(gavin.sharp)
Attachment #8530413 -
Flags: review?(mhammond)
Comment 4•9 years ago
|
||
Comment on attachment 8530413 [details] [diff] [review] patch Review of attachment 8530413 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. > Mark: if you could help write a test for this, I'd appreciate it. So far I've failed to work out how to do this - as the function uses getDefaultBranch(), manually setting the default pref before the test doesn't have the desired effect. If I try and set the pref using the default branch, something breaks in the pref service when run under xpcshell - subsequent gets of the value return NS_ERROR_MALFORMED_URI (sp?). I didn't dig too deep here though, so maybe that's somewhat shallow.
Attachment #8530413 -
Flags: review?(mhammond) → review+
Comment 6•9 years ago
|
||
Comment on attachment 8530413 [details] [diff] [review] patch Approval Request Comment [Feature/regressing bug #]: Bug 1102416 [User impact if declined]: Certain params in search engine queries may have the wrong value [Describe test coverage new/current, TBPL]: Existing tests pass, but no new tests specifically for this case. [Risks and why]: Low risk, limited to those params *still* not working correctly [String/UUID change made/needed]: None
Attachment #8530413 -
Flags: approval-mozilla-beta?
Attachment #8530413 -
Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/beba124659ca
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Updated•9 years ago
|
status-firefox37:
--- → fixed
Updated•9 years ago
|
Attachment #8530413 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•9 years ago
|
Attachment #8530413 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 8•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/cbc303991308 https://hg.mozilla.org/releases/mozilla-beta/rev/baded7123fe8
You need to log in
before you can comment on or make changes to this bug.
Description
•