Only re-initialize nsSearchService on locale change in Fennec

VERIFIED FIXED in Firefox 32

Status

()

Firefox
Search
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: Gavin, Assigned: rnewman)

Tracking

32 Branch
Firefox 34
Points:
---
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(firefox32+ verified, firefox33+ verified, firefox34+ verified)

Details

Attachments

(1 attachment)

See bug 1018240 comment 21.
Flags: firefox-backlog+
(Assignee)

Updated

4 years ago
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
(Assignee)

Comment 1

4 years ago
Created attachment 8462233 [details] [diff] [review]
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.
Attachment #8462233 - Flags: review?(adw)
status-firefox32: --- → affected
status-firefox33: --- → affected
status-firefox34: --- → affected
tracking-firefox32: --- → +
tracking-firefox33: --- → +
tracking-firefox34: --- → +
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)
(Assignee)

Comment 5

4 years ago
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
Last Resolved: 4 years ago
status-firefox34: affected → fixed
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+

Updated

4 years ago
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)

Updated

4 years ago
Iteration: 34.1 → 34.2
(Assignee)

Comment 9

4 years ago
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)

Updated

4 years ago
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

Updated

4 years ago
Status: RESOLVED → VERIFIED
QA Whiteboard: [qa+] → [qa!]
status-firefox32: fixed → verified
status-firefox33: fixed → verified
status-firefox34: fixed → verified
You need to log in before you can comment on or make changes to this bug.