1352731 - allow localisation of messages from nsIMsgFilter::LogRuleHitFail()

Status: RESOLVED FIXED

(MailNews Core :: Filters, defect)

Description

[:aceman]

In bug 1350607 it was found that the messages printed out by nsIMsgFilter::LogRuleHitFail() are hardcoded in the C++ code in English and can't be translated.
A typical use looks like this:
filter->LogRuleHitFail(filterAction, msgHdr, err, "Move failed");

However, these messages are shown to the user e.g. via the filter log file, where all other messages are translated.
Even worse, the sentence containing these messages is translated:
"Filter Action Failed: "%1$S" with error code=%2$S while attempting:"
%1$S is replaced by the message sent to LogRuleHitFail (which is not translated).

I propose to somehow fix this.

Comment 1

[:aceman]

Attachment: WIP patch

This would be the easiest solution, to just pass stringID to the function and let it fetch the string from filter.properties.

However, I can guess the function was made to take the raw string to allow extension-created custom filters and messages to be passed in. Extensions probably can't pass in stringIDs so they need to pass the final string.

I'd just like you to confirm this assumption and I can rework the patch. Then the callers would need to fetch the localised strings and pass it to the function.

Comment 2

[:aceman]

For the record, there are no callers of logrulehit* in extensions indexed in dxr/addons.

Comment 3

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #2)
> For the record, there are no callers of logrulehit* in extensions indexed in
> dxr/addons.
So what's stopping you then? r?jorgk ;-)

Comment 4

[:aceman]

I don't want to drop an intentional feature. If we still allow custom filter rules/actions, we may need to allow custom failure messages to be output.

Comment 5

[Onno Ekker [:nONoNonO UTC+1]]

My last use of custom filters in an add-on was with the Forward add-on and I added custom filter actions to it in 2012. As far as I understood it, the process is somewhat crude, since you can't just overlay some xul or call an API, but have to add the actions yourself using JavaScript. This means for instance that it's hard (or impossible?) to add the custom filter action on a specific position in the list. The custom filters added by the Forward add-on would fit better directly under the original Forward filter action, much like the Forward As menu is directly under the Forward menu item, but then I didn't see a way to accomplish this.

That being said, I haven't used LogRuleHitFail, but from the looks of it, it should be fairly straightforward for add-on developer's to pass their own localized error message in the call.

Comment 6

[:aceman]

Attachment: 1352731.patch

OK, then we should keep the possibility to send the string that is to be output.

What about this version? For out internal actions, the passed string can be a stringID to look up in filter.properties (to not have to instantiate filter.properties bundle at all callers, only inside logRuleHitGeneric() where it already is). Still, if the stringID is not found, it will be output verbatim.
If an addon calls this function with a localized string for a custom action, it will be output verbatim.

Comment 7

[Magnus Melin [:mkmelin]]

Comment on attachment 9042847 [details] [diff] [review]
1352731.patch

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

Looks ok to me. r=mkmelin

Comment 8

[:aceman]

Thanks.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a0a1515e9098
localize messages from nsIMsgFolder.logRuleHitFail(). r=mkmelin

Comment 10

[:aceman]

Attachment: 1352731-SM.patch

Sorry, I forgot the Seamonkey strings here.
While the logging should not outright fail due to this, it should log the stringIDs instead of the (translated) strings.

Comment 11

[Ian Neal]

Comment on attachment 9061932 [details] [diff] [review]
1352731-SM.patch

r=me
thanks

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/52dffc60462b
add missed strings for failed filter actions to SeaMonkey. r=IanN