Closed Bug 937870 Opened 7 years ago Closed 7 years ago

nsSearchService displays duplicate engine prompt regardless of confirmation set by caller

Categories

(Firefox :: Search, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 28

People

(Reporter: liuche, Assigned: liuche)

References

Details

Attachments

(1 file, 1 obsolete file)

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/search/nsSearchService.js#1475

The prompt for adding a duplicate engine should only be displayed if the caller sets confirm = true. Implementation also doesn't match comment.
Attachment #831117 - Flags: review?(gavin.sharp)
Comment on attachment 831117 [details] [diff] [review]
Patch: check confirmation for duplicate engine prompt

The !engineToUpdate check needs to stay there, since we allow "duplicate" installs in that case (the comment should probably be updated too).

Looks like http://hg.mozilla.org/mozilla-central/rev/e9b946da20cb (bug 493051) regressed this, there should be an additional _confirm check before calling promptError.
Attachment #831117 - Flags: review?(gavin.sharp) → review-
BTW, while you're fixing search stuff, maybe you're interested in fixing bug 863474? :)

(I imagine it could be useful for Android since it'd let you do a custom prompt.)
Blocks: 493051
Component: General → Search
Product: Toolkit → Firefox
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #2)
> there should be an additional if (_confirm) check before
> calling promptError.

Oh, and in the !_confirm case, we need to call onError() instead.
Thanks for the suggestions! Updated the comments as well.
Attachment #831117 - Attachment is obsolete: true
Attachment #831155 - Flags: review?(gavin.sharp)
Comment on attachment 831155 [details] [diff] [review]
Patch: check confirmation for duplicate engine prompt v2

Thanks a lot!
Attachment #831155 - Flags: review?(gavin.sharp) → review+
Assignee: nobody → liuche
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/bd3fb665acfc
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
Depends on: 1222107
You need to log in before you can comment on or make changes to this bug.