Opened on 01/22/2015 at 02:52:20 AM
Closed on 02/19/2018 at 01:41:40 PM
Last modified on 03/22/2018 at 01:53:44 PM
#1846 closed change (fixed)
Remove the locale parameter of Notification.getLocalizedTexts
| Reported by: | fhd | Assignee: | Yael |
|---|---|---|---|
| Priority: | P4 | Milestone: | |
| Module: | Core | Keywords: | goodfirstbug |
| Cc: | kzar, sergz, greiner, Ross | Blocked By: | |
| Blocking: | Platform: | Unknown | |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): | |||
Description (last modified by fhd)
Background
Notification.getLocalizedTexts currently has a locale parameter that makes it possible to retrieve the text for any locale, not just the application locale. This is completely unused (other than in tests), and there's no real point in getting texts for a different language (falling back to the default locale for missing texts already happens automatically).
What to change
Remove the locale parameter from Notification.getLocalizedTexts.
Attachments (0)
Change History (7)
comment:2 Changed on 01/04/2018 at 11:05:10 AM by sergz
- Keywords goodfirstbug added
- Ready set
- Tester set to Unknown
- Verified working unset
comment:3 Changed on 02/19/2018 at 01:36:32 PM by abpbot
comment:4 Changed on 02/19/2018 at 01:41:40 PM by kzar
- Cc kzar sergz fhd greiner Ross added
- Priority changed from Unknown to P4
- Resolution set to fixed
- Review URL(s) modified (diff)
- Status changed from new to closed
Sergz and I reviewed the change, I've checked it still lints and the unit tests pass. I couldn't figure out how to trigger this code path for real however, anyone have any ideas how? I'd like to update the Hints for Testers section.
comment:5 Changed on 02/19/2018 at 01:41:54 PM by kzar
- Cc fhd removed
comment:6 Changed on 02/19/2018 at 02:10:00 PM by kzar
- Owner set to Yael
comment:7 Changed on 03/22/2018 at 01:53:44 PM by Ross
- Tester changed from Unknown to Ross
- Verified working set
Done. I tested this by checking that notifications were localized and that the fallback to en-US still works as expected.
ABP 3.0.2.1983
Chrome 49 / 65 / Windows 10
Opera 36 / 51 / Windows 7
A commit referencing this issue has landed:
Issue 1846 - Remove the locale parameter of Notification.getLocalizedTexts