Remove nsLocaleService, nsILocaleService, nsLocale, and nsPosixLocale

RESOLVED FIXED in Firefox 57

Status

()

P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: gandalf, Assigned: gandalf)

Tracking

(Blocks: 1 bug)

unspecified
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(1 attachment)

We're getting very close to be able to remove those locales.

Once 57 hits and we finish transitioning away a few last files, we should be ready.
(Assignee)

Updated

2 years ago
Blocks: 1347507
Priority: -- → P3
After fixing bug 1356503, we can remove nsWin32Locale too.
(Assignee)

Comment 2

2 years ago
Jorg, what can I do to minimize the disruption for Tb with regards to this bug and bug 1313625?
Flags: needinfo?(jorgk)

Comment 3

2 years ago
We have three bugs going to remove the use of nsIScriptableDateFormat: Bug 1346549, bug 1313659 and bug 1229684. All have patches pending review. So with some luck we can land those patches in the next 10 days. When do you plan to remove it?

I know nothing about nsLocaleService, nsILocaleService, nsLocale, and nsPosixLocale. Do we use that in comm-central? What do I look for?

I've done some spot checks:
https://dxr.mozilla.org/comm-central/search?q=getApplicationLocale
https://dxr.mozilla.org/comm-central/search?q=getSystemLocale
Yes, there are some uses. What are the equivalents?
Flags: needinfo?(jorgk)

Updated

2 years ago
Depends on: 1357822
(Assignee)

Comment 4

2 years ago
> When do you plan to remove it?

Right after 56 branches off so I guess in 3 months? I just want to make sure we don't cause any troubles :)

> https://dxr.mozilla.org/comm-central/search?q=getApplicationLocale

You seem to only have one use of this in StringBundle.js. You should switch to Services.locale.getAppLocaleAsLangTag.


> https://dxr.mozilla.org/comm-central/search?q=getSystemLocale

You seem to really only have one use of this in navigator.js - you should switch it to mozIOSPreferences systemLocale attribute.
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1337076
(Assignee)

Updated

2 years ago
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Summary: Deprecate and remove nsLocaleService, nsILocaleService, nsLocale, and nsPosixLocale → Remove nsLocaleService, nsILocaleService, nsLocale, and nsPosixLocale
Comment hidden (mozreview-request)

Comment 8

2 years ago
mozreview-review
Comment on attachment 8893282 [details]
Bug 1356263 - Remove nsLocaleService, nsILocaleService and ns*Locale APIs.

https://reviewboard.mozilla.org/r/164338/#review169698

::: dom/media/webspeech/synth/nsSynthVoiceRegistry.cpp:625
(Diff revision 2)
> -  rv = localeService->GetLocaleComponentForUserAgent(uiLang);
> -  if (NS_WARN_IF(NS_FAILED(rv))) {
> -    return nullptr;
> -  }
>  
> -  if (FindVoiceByLang(uiLang, &retval)) {
> +  if (FindVoiceByLang(NS_ConvertUTF8toUTF16(uiLang), &retval)) {

uiLang is ASCII character, so we should use NS_ConvertASCIItoUTF16 instead.

::: intl/locale/unix/moz.build:7
(Diff revision 2)
>  
> -SOURCES += [
> -    'nsPosixLocale.cpp',
> -]
> -
>  if CONFIG['MOZ_WIDGET_TOOLKIT'] != 'android':

intl/locale/unix uses GTK build only now, so we can remove android condition and modify intl/locale/moz.build not to traverse this directory on android build.

::: intl/locale/windows/moz.build
(Diff revision 2)
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
> -SOURCES += [
> -    'nsWin32Locale.cpp',
> -    'nsWinCharset.cpp',

We don't resolve bug 943272.  So we cannot remove nsWinCharset.cpp that is nsIPlatformCharset implementation yet.

See http://searchfox.org/mozilla-central/rev/f0e4ae5f8c40ba742214e89aba3f554da0b89a33/netwerk/streamconv/converters/nsIndexedToHTML.cpp#511 and bug 943284

::: intl/locale/windows/moz.build
(Diff revision 2)
> -GENERATED_FILES = [
> -    'wincharset.properties.h',
> -]
> -wincharset = GENERATED_FILES['wincharset.properties.h']
> -wincharset.script = '../props2arrays.py'
> -wincharset.inputs = ['wincharset.properties']

This depends on nsWinCharset.cpp, so don't remove this until bug 943284 is fixed.
Attachment #8893282 - Flags: review?(m_kato)
Comment hidden (mozreview-request)
(Assignee)

Comment 10

2 years ago
mozreview-review-reply
Comment on attachment 8893282 [details]
Bug 1356263 - Remove nsLocaleService, nsILocaleService and ns*Locale APIs.

https://reviewboard.mozilla.org/r/164338/#review169698

> intl/locale/unix uses GTK build only now, so we can remove android condition and modify intl/locale/moz.build not to traverse this directory on android build.

I would prefer to do the android+icu cleanup separately if that's ok with you.
Comment hidden (mozreview-request)

Comment 12

2 years ago
mozreview-review
Comment on attachment 8893282 [details]
Bug 1356263 - Remove nsLocaleService, nsILocaleService and ns*Locale APIs.

https://reviewboard.mozilla.org/r/164338/#review170180
Attachment #8893282 - Flags: review?(m_kato) → review+
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #10)
> Comment on attachment 8893282 [details]
> Bug 1356263 - Remove nsLocaleService, nsILocaleService and ns*Locale APIs.
> 
> https://reviewboard.mozilla.org/r/164338/#review169698
> 
> > intl/locale/unix uses GTK build only now, so we can remove android condition and modify intl/locale/moz.build not to traverse this directory on android build.
> 
> I would prefer to do the android+icu cleanup separately if that's ok with
> you.

OK, we should handle another bug, not this bug.
(Assignee)

Comment 14

2 years ago
Filed bug 1387332. Will work on it next week.

Comment 15

2 years ago
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6f029af6aa34
Remove nsLocaleService, nsILocaleService and ns*Locale APIs. r=m_kato
Duplicate of this bug: 1350102
Duplicate of this bug: 1370142
https://hg.mozilla.org/mozilla-central/rev/6f029af6aa34
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.