Closed Bug 1356263 Opened 6 years ago Closed 6 years ago

Remove nsLocaleService, nsILocaleService, nsLocale, and nsPosixLocale

Categories

(Core :: Internationalization, enhancement, P3)

Product:
Core
Component:
Internationalization
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Bug 1356263 - Remove nsLocaleService, nsILocaleService and ns*Locale APIs.
59 bytes, text/x-review-board-request
m_kato
: review+
Details 
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.
Priority: -- → P3
After fixing bug 1356503, we can remove nsWin32Locale too.
Jorg, what can I do to minimize the disruption for Tb with regards to this bug and bug 1313625?
Flags: needinfo?(jorgk)
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)
Depends on: 1357822
> 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: nobody → gandalf
Status: NEW → ASSIGNED
Summary: Deprecate and remove nsLocaleService, nsILocaleService, nsLocale, and nsPosixLocale → Remove nsLocaleService, nsILocaleService, nsLocale, and nsPosixLocale
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 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 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.
Filed bug 1387332. Will work on it next week.
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6f029af6aa34
Remove nsLocaleService, nsILocaleService and ns*Locale APIs. r=m_kato
https://hg.mozilla.org/mozilla-central/rev/6f029af6aa34
Status: ASSIGNED → RESOLVED
Closed: 6 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.