1617371 - comm/mailnews/base/src/nsMailDirProvider.cpp:170:22: error: no member named 'GetSelectedLocale' in 'nsIXULChromeRegistry'

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Richard Marti (:Paenglab)]

Most probably busted by bug 1616912. A build with the parent changeset is successful locally.

Comment 1

[Kai Engert (:KaiE:)]

Apparently the intention of that other bug was to remove a parameter. But a whole function was removed from IDL. Apparently we need that functionality? I asked in the bug if the function could be restored, then we'd simply have to adjust the call to remove a parameter.

Comment 2

[Kai Engert (:KaiE:)]

I see two callers for the removed API, nsMsgMailSession::GetSelectedLocaleDataDir and nsMailDirProvider::AppendingEnumerator::AppendingEnumerator

Who knows if we still need the selected locale infomation, or if those calls could simply be removed?

Comment 3

[Kai Engert (:KaiE:)]

We have the following options, but I don't know which one is the right approach.

Option 1:
If we still need the selected locale information, ask Firefox devs to restore the call, and adopt.

Option 2:
If we don't need the information any longer, then remove the code that attempts to use it.

Option 3:
If we still need the selected locale information, figure out a different mechanism to obtain it.

Comment 4

[Kai Engert (:KaiE:)]

Attachment: 1617371-option1-gecko-restore.patch

Comment 5

[Kai Engert (:KaiE:)]

Attachment: 1617371-option1-comm-adjust.patch

Comment 6

[Kai Engert (:KaiE:)]

Attachment: 1617371-option2-comm-remove.patch

Comment 7

[Kai Engert (:KaiE:)]

I've prepared patches for option 1 and option 2.

Option 2 is the easiest way to fix the build bustage, but I don't know if it breaks functionality.

Comment 8

[Kai Engert (:KaiE:)]

Comment on attachment 9128321 [details] [diff] [review]
1617371-option2-comm-remove.patch

This patch builds, and basic email functionality still works. Also ./mach package worked.

Should we use this patch (at least temporarily) to unbreak the build?

Comment 9

[Magnus Melin [:mkmelin]]

Comment on attachment 9128321 [details] [diff] [review]
1617371-option2-comm-remove.patch

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

Looks like this is not code we need. We also don't ship providers in isp anymore... and apparently this allowed locale specific extensions in there https://searchfox.org/comm-central/rev/631e8b52544ba72abc8a1ab9551a57f3a6441960/mailnews/base/src/nsMailDirProvider.cpp#87-90

Comment 10

[Magnus Melin [:mkmelin]]

Comment on attachment 9128321 [details] [diff] [review]
1617371-option2-comm-remove.patch

Please go ahead and land

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f141b2e4803f
Remove callers to GetSelectedLocale. r=mkmelin

Comment 12

[Kai Engert (:KaiE:)]

Gandalf suggested, if we still need that information, we might be able to use LocaleService.

Comment 13

[Kai Engert (:KaiE:)]

We'll need additional fixes. Lots of failed Javascript calls in tests.

Comment 14

[Magnus Melin [:mkmelin]]

Attachment: bug1617371_js_part.patch

Maybe this. Will have to rebuild to verify.
(This code should very likely just be removed later, with it's potential callers!)

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/95dc8118d884
fix locale lookup for mozmill.jsm. rs=bustage-fix