Closed Bug 1164698 Opened 9 years ago Closed 9 years ago

Update Pocket code to latest version (May 13th code drop)

Categories

(Firefox :: Pocket, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 41
Tracking Status
firefox38.0.5 --- fixed
firefox39 --- fixed
firefox40 --- fixed
firefox41 --- fixed

People

(Reporter: nate+bugzilla, Assigned: nate+bugzilla)

Details

Attachments

(1 file, 3 obsolete files)

Attached patch WedsDrop.patch (obsolete) — Splinter Review
Small code drop with minor tweaks to FxA links and a fix for navigator issue referenced in bug 1164407
Attachment #8605547 - Flags: review?(michael)
Attachment #8605547 - Flags: review?(jaws)
Attachment #8605547 - Flags: review?(dolske)
Attached patch WedsDropv2.patch (obsolete) — Splinter Review
Attachment #8605547 - Attachment is obsolete: true
Attachment #8605547 - Flags: review?(michael)
Attachment #8605547 - Flags: review?(jaws)
Attachment #8605547 - Flags: review?(dolske)
Attachment #8605565 - Flags: review?(dolske)
Attached patch WedsDropv3.patch (obsolete) — Splinter Review
- Fixed typo
- Fixed patch (second one didn't include both files)
Attachment #8605566 - Flags: review?(dolske)
Attached patch WedsDropv3.patchSplinter Review
- Fixed typo
- Fixed patch (second one didn't include both files)
- (And this time) marked the other files as obsolete

Sorry for the comment spam
Attachment #8605565 - Attachment is obsolete: true
Attachment #8605566 - Attachment is obsolete: true
Attachment #8605565 - Flags: review?(dolske)
Attachment #8605566 - Flags: review?(dolske)
Attachment #8605568 - Flags: review?(dolske)
Comment on attachment 8605568 [details] [diff] [review]
WedsDropv3.patch

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

::: browser/components/pocket/pktApi.js
@@ +211,5 @@
>          var url = baseAPIUrl + options.path;
>          var data = options.data || {};
> +        data.locale_lang = Cc["@mozilla.org/chrome/chrome-registry;1"].
> +             getService(Ci.nsIXULChromeRegistry).
> +             getSelectedLocale("browser");

Followup from bug 1164407: you'll need to deal with the "ja-JP-mac" locale issue from that bug.

I think you can just add the same hack as in that bug.

  if (locale == "ja-JP-mac")
    locale = "ja";

Or I suppose you could instead add a "Translations.ja-JP-mac = Translations.ja" at the end of dictionary.js.
Attachment #8605568 - Flags: review?(dolske) → review-
Dolske: Per our conversation in IRC and my additional testing:

- The dictionary loading only takes the prefix (ex 'ja' or 'ru') so even if locale_lang is 'ja' or 'ja-JP' or 'ja-JP-mac', it should work. Relevant bits: http://mxr.mozilla.org/mozilla-central/source/browser/components/pocket/panels/js/saved.js#462

- Separately, I just tested hard coding those various strings as outputs of the UILocale method and everything worked as expected for all languages. 

- As we talked about, when we eventually ship more specific languages like en-US vs en-GB, we'll need to update the regex in the following link but I've filed a internal ticket and we'll tackle when the time comes: http://mxr.mozilla.org/mozilla-central/source/browser/components/pocket/panels/js/saved.js#640

Given that, I think we should be good.
Flags: needinfo?(dolske)
Comment on attachment 8605568 [details] [diff] [review]
WedsDropv3.patch

Concur!
Flags: needinfo?(dolske)
Attachment #8605568 - Flags: review- → review+
Comment on attachment 8605568 [details] [diff] [review]
WedsDropv3.patch

[Triage Comment]

a+ for aurora/beta/release: required for Pocket launch in 38.0.5.
Attachment #8605568 - Flags: approval-mozilla-release+
Attachment #8605568 - Flags: approval-mozilla-beta+
Attachment #8605568 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/db6a6012c872
Assignee: nobody → nate
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.