1043627 - Only re-initialize nsSearchService on locale change in Fennec

Status: VERIFIED FIXED

(Firefox :: Search, defect)

Description

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

See bug 1018240 comment 21.

Comment 1

[Richard Newman [:rnewman]]

Attachment: Only re-initialize nsSearchService on locale change in Fennec. v1

I elected to conditionalize on MOZ_FENNEC (added in Bug 797613), rather than
ANDROID as used elsewhere in the file, to avoid this code being active in B2G.

This removes the registration of the pref observer, and also the handler for
changes in the observed pref.

Comment 2

[Drew Willcoxon :adw]

Comment on attachment 8462233 [details] [diff] [review]
Only re-initialize nsSearchService on locale change in Fennec. v1

Review of attachment 8462233 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks.

Comment 3

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/fx-team/rev/7e44cabb92bc

Comment 4

[Florian Quèze [:florian]]

(:Gavin Sharp from bug 1018240 comment #21)
> Oops - I wanted to look at this before it landed (hence the feedback), and I
> missed that being cleared.
> 
> Can we make this re-init behavior Fennec-only? Reinitialization is a bit
> scary to me and I'd prefer if it not happen on Desktop, where there little
> value.

This re-init behavior is very useful for tests, see bug 1029148. If you don't want this behavior to happen on Desktop by default, maybe we could put it behind a pref instead of ifdef'ing it out?

Comment 5

[Richard Newman [:rnewman]]

Comment on attachment 8462233 [details] [diff] [review]
Only re-initialize nsSearchService on locale change in Fennec. v1

Follow-up approval alongside Bug 1018240.

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7e44cabb92bc

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a439e9a51f95
https://hg.mozilla.org/releases/mozilla-beta/rev/61b30b605194

Comment 8

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

(In reply to Florian Quèze [:florian] [:flo] from comment #4)
> This re-init behavior is very useful for tests, see bug 1029148. If you
> don't want this behavior to happen on Desktop by default, maybe we could put
> it behind a pref instead of ifdef'ing it out?

Yes - we can put it behind a pref. Too late here - can you file another bug?

Comment 9

[Richard Newman [:rnewman]]

MarcoM: what's up with the iteration spam on this bug? It was fixed and uplifted a week ago. Just want to make sure there isn't a bug in your scripts.

Comment 10

[Marco Mucci [:MarcoM]]

Waiting on QA to verify it as it is marked [qa+]

(In reply to Richard Newman [:rnewman] from comment #9)
> MarcoM: what's up with the iteration spam on this bug? It was fixed and
> uplifted a week ago. Just want to make sure there isn't a bug in your
> scripts.

Comment 11

[Marco Mucci [:MarcoM]]

Hi Liz, can a QA contact be assigned for verification of this bug.