Closed Bug 1234673 Opened 4 years ago Closed 4 years ago

Avoid users from locales ar, fa, he, ur for e10s rollout to beta

Categories

(Firefox :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 46
Tracking Status
e10s m8+ ---
firefox45 + fixed
firefox46 --- fixed

People

(Reporter: jimm, Assigned: Felipe)

References

Details

Attachments

(1 file, 3 obsolete files)

due to M9 Bug 1033488 - [e10s] Do bidi state detection in the parent process on Window
Blocks: e10s-beta
Assignee: nobody → gkrizsanits
I'll take this one
Assignee: gkrizsanits → felipc
Status: NEW → ASSIGNED
[Tracking Requested - why for this release]:
we need to have this on for the e10s beta45 experiment
Attached patch patch (obsolete) — Splinter Review
Attachment #8708513 - Flags: review?(jmathies)
Comment on attachment 8708513 [details] [diff] [review]
patch

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

::: toolkit/xre/nsAppRunner.cpp
@@ +4732,5 @@
> +
> +    if (localeStr.EqualsLiteral("ar") ||
> +        localeStr.EqualsLiteral("fa") ||
> +        localeStr.EqualsLiteral("he") ||
> +        localeStr.EqualsLiteral("en-US") ||

oops, ignore the en-US that I used for testing..

And I'm waiting for confirmation from Pike about whether "ur" is correct.
Tracking to make sure it is fixed before 45 beta 1.
Attachment #8708513 - Flags: review?(jmathies) → review+
Attached patch update histogram (obsolete) — Splinter Review
Hopefully an enumerated histogram can be updated on the fly..
Attachment #8709513 - Flags: review?(vladan.bugzilla)
Attached patch update histogram (obsolete) — Splinter Review
and actually, since it's being updated, let's also include together the DISABLED_FOR_ADDONS option for bug 1234675
Attachment #8709513 - Attachment is obsolete: true
Attachment #8709513 - Flags: review?(vladan.bugzilla)
Attachment #8709515 - Flags: review?(vladan.bugzilla)
Whiteboard: [e10s-45-uplift]
Comment on attachment 8709515 [details] [diff] [review]
update histogram

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

::: toolkit/components/telemetry/Histograms.json
@@ +8306,5 @@
>    "E10S_AUTOSTART_STATUS": {
>      "expires_in_version": "never",
>      "kind": "enumerated",
> +    "n_values": 8,
> +    "description": "Why e10s is enabled or disabled (0=ENABLED_BY_USER, 1=ENABLED_BY_DEFAULT, 2=DISABLED_BY_USER, 3=DISABLED_IN_SAFE_MODE, 4=DISABLED_FOR_ACCESSIBILITY, 5=DISABLED_FOR_MAC_GFX, 6=DISABLED_FOR_BIDI, 7=DISABLED_FOR_ADDONS)"

you can't change a histogram's bucketing (including n_values) after it has been released, it confuses the aggregator :(
you'll have to declare a new histogram, and leave enough room to grow in n_values

also add the alert_emails and bug_numbers fields
Attachment #8709515 - Flags: review?(vladan.bugzilla) → review-
Depends on: 1241294
Comment on attachment 8709515 [details] [diff] [review]
update histogram

I moved this patch to bug 1241294
Attachment #8709515 - Attachment is obsolete: true
Approval Request Comment
[Feature/regressing bug #]: due to some issues with text input on these locales on e10s, e10s should be disabled for users of these locales (both on release and on the beta45 experiment).
The proper fix for these locales is not marked as an e10s release blocker for other locales. See https://bugzilla.mozilla.org/show_bug.cgi?id=1033488#c11
[User impact if declined]: users of these locales can be exposed to e10s bugs.
[Describe test coverage new/current, TreeHerder]: landed on inbound and this will be part of the test plan that the QA team will do for the e10s experiment
[Risks and why]: accidentally disabling e10s on other locales
[String/UUID change made/needed]: none
Attachment #8708513 - Attachment is obsolete: true
Attachment #8710218 - Flags: review+
Attachment #8710218 - Flags: approval-mozilla-aurora?
Comment on attachment 8710218 [details] [diff] [review]
patch as landed, r=jimm

ok, needed for the e10s experiment
Attachment #8710218 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/ef6dc593fccc
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Blocks: 1033488
nsILocale getApplicationLocale():
    /*
     * NOTE: This has nothing to do with the locale used for localization of
     * the application (UI text strings etc.). This method returns something
     * similar to getSystemLocale.
     *
     * @return User's OS setting for preferred locale.
     */

*facepalm
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Since this was tracking, I'll handle the fix in a follow-up
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Blocks: 1243882
[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel	beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS				Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected
Blocks: e10s-rtl
You need to log in before you can comment on or make changes to this bug.