Closed Bug 1043627 Opened 5 years ago Closed 5 years ago

Only re-initialize nsSearchService on locale change in Fennec

Categories

(Firefox :: Search, defect)

32 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 34
Iteration:
34.2
Tracking Status
firefox32 + verified
firefox33 + verified
firefox34 + verified

People

(Reporter: Gavin, Assigned: rnewman)

References

Details

Attachments

(1 file)

See bug 1018240 comment 21.
Flags: firefox-backlog+
Assignee: nobody → rnewman
Blocks: 1018240
Status: NEW → ASSIGNED
Summary: make search service locale reinitialization fennec-only → Only re-initialize nsSearchService on locale change in Fennec
Version: 29 Branch → 32 Branch
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.
Attachment #8462233 - Flags: review?(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.
Attachment #8462233 - Flags: review?(adw) → review+
(: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?
Flags: needinfo?(gavin.sharp)
Comment on attachment 8462233 [details] [diff] [review]
Only re-initialize nsSearchService on locale change in Fennec. v1

Follow-up approval alongside Bug 1018240.
Attachment #8462233 - Flags: approval-mozilla-beta?
Attachment #8462233 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/7e44cabb92bc
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Attachment #8462233 - Flags: approval-mozilla-beta?
Attachment #8462233 - Flags: approval-mozilla-beta+
Attachment #8462233 - Flags: approval-mozilla-aurora?
Attachment #8462233 - Flags: approval-mozilla-aurora+
Iteration: --- → 34.1
QA Whiteboard: [qa?]
QA Whiteboard: [qa?] → [qa+]
QA Contact: aaron.train
(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?
Flags: needinfo?(gavin.sharp)
Iteration: 34.1 → 34.2
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.
Flags: needinfo?(mmucci)
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.
Flags: needinfo?(mmucci)
QA Contact: aaron.train
Hi Liz, can a QA contact be assigned for verification of this bug.
Flags: needinfo?(lhenry)
Flags: needinfo?(lhenry)
QA Contact: aaron.train
Status: RESOLVED → VERIFIED
QA Whiteboard: [qa+] → [qa!]
You need to log in before you can comment on or make changes to this bug.