Closed Bug 1383463 Opened 2 years ago Closed 2 years ago

Migrate Intl.DateTimeFormat to mozIntl.DateTimeFormat in chrome - take 2, cookies, page info

Categories

(Core :: Internationalization, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox55 --- fixed
firefox56 --- fixed

People

(Reporter: jorgk-bmo, Assigned: jorgk-bmo)

References

Details

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #1354445 +++

Looks like bug 1354445 forgot (at least):
browser/components/preferences/cookies.js
browser/base/content/pageinfo/pageInfo.js
It's been a while since I fixed a FF bug ;-)
Attachment #8889098 - Flags: review?(gandalf)
Comment on attachment 8889098 [details] [diff] [review]
1383463-cookies-page-info-mozIntl.patch (v1).

sweet. Looks good to me :)
Attachment #8889098 - Flags: review?(gandalf) → review+
Actually, that indentation should be like this to match
https://hg.mozilla.org/mozilla-central/rev/79673e659b31#l1.33

Should I do a try push? If so, with what parameters? Is this likely to upset anything?
Attachment #8889098 - Attachment is obsolete: true
Flags: needinfo?(gandalf)
Attachment #8889134 - Flags: review+
Assignee: nobody → jorgk
(In reply to Jorg K (GMT+2) from comment #3)
> Should I do a try push? If so, with what parameters? Is this likely to upset
> anything?

I don't think so. I believe it's enough to manually test if the date is displayed properly in those two UIs.
Flags: needinfo?(gandalf)
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #4)
> I don't think so. I believe it's enough to manually test if the date is
> displayed properly in those two UIs.
Works as expected.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d14ed20c131
Use mozIntl for cookies and page info. r=zibi
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/3d14ed20c131
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment on attachment 8889134 [details] [diff] [review]
1383463-cookies-page-info-mozIntl.patch (v1b).

Approval Request Comment
[Feature/Bug causing the regression]: This is part of using mozIntl-formatted dates/times everywhere in the UI. Two spots were forgotten in bug 1354445. So this is a follow-up.
[User impact if declined]: Users of the en-US version in regions where a localised version isn't available (Australia, India) will get inconsistent date/time displays in the UI. 
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]:
No, but the test is to display page info and cookie expiry date and check that those dates have the same format as, for example, "Last Modified" for bookmarks. Needs to be checked with date format set to en-GB in regional settings on Windows.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Simple JS change done many times in bug 1354445.
[String changes made/needed]: None.
Attachment #8889134 - Flags: approval-mozilla-beta?
Comment on attachment 8889134 [details] [diff] [review]
1383463-cookies-page-info-mozIntl.patch (v1b).

followup for bug 1354445, localized date formatting for cookies and pageinfo.  beta55+

should be in 55.0b13
Attachment #8889134 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Jorg K (GMT+2) from comment #8)
> [Is this code covered by automated tests?]: No.
> [Has the fix been verified in Nightly?]: Yes.
> [Needs manual test from QE? If yes, steps to reproduce]:
> No, but the test is to display page info and cookie expiry date and check
> that those dates have the same format as, for example, "Last Modified" for
> bookmarks. Needs to be checked with date format set to en-GB in regional
> settings on Windows.

Setting qe-verify- based on Jorg K's assessment on manual testing needs and the fact that this fix has already been verified by him on Nightly.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.