937870 - nsSearchService displays duplicate engine prompt regardless of confirmation set by caller

Status: RESOLVED FIXED

(Firefox :: Search, defect)

Description

[Chenxia Liu [:liuche]]

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.

Comment 1

[Chenxia Liu [:liuche]]

Attachment: Patch: check confirmation for duplicate engine prompt

Comment 2

[:Gavin Sharp [email: [e-mail]]]

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.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

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.)

Comment 4

[:Gavin Sharp [email: [e-mail]]]

(In reply to :Gavin Sharp (email [e-mail]) 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.

Comment 5

[Chenxia Liu [:liuche]]

Attachment: Patch: check confirmation for duplicate engine prompt v2

Thanks for the suggestions! Updated the comments as well.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 831155 [details] [diff] [review]
Patch: check confirmation for duplicate engine prompt v2

Thanks a lot!

Comment 7

[Chenxia Liu [:liuche]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bd3fb665acfc

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/bd3fb665acfc