1368646 - Port bug 1368418 |Remove some unused i18n modules| to mailnews

Status: RESOLVED FIXED

(MailNews Core :: Internationalization, enhancement)

Description

[Jorg K (CEST = GMT+2)]

+++ This bug was initially created as a clone of Bug #1368418 +++

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

I guess you only need to port nsISemanticUnitScanner and its subclass, which is just the first patch of bug 1368418.

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: lwbrk.xpt.patch - bustage fix

Landed this as a bustage fix:
https://hg.mozilla.org/comm-central/rev/a5afc78bc5d86c9d4280f2955369ff82e3fbab9d

Aceman, would you be able to port the removed code to mailnews:
https://hg.mozilla.org/mozilla-central/rev/c14a7823ce37ca55fc78e0d7e970e8ee764fcd80#l5.2

So instead of being busted, I've just broken the Bayesian filter :-(

Comment 3

[Jorg K (CEST = GMT+2)]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #1)
> I guess you only need to port nsISemanticUnitScanner and its subclass, which
> is just the first patch of bug 1368418.
We just need the Start() and Next() functions? Without all the infrastructure, IDL, etc. right?
Although of course, things like NextWord() come from nsSampleWordBreaker.

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Jorg K (GMT+2) from comment #3)
> (In reply to Xidorn Quan [:xidorn] UTC+10 from comment #1)
> > I guess you only need to port nsISemanticUnitScanner and its subclass, which
> > is just the first patch of bug 1368418.
> We just need the Start() and Next() functions? Without all the
> infrastructure, IDL, etc. right?
> Although of course, things like NextWord() come from nsSampleWordBreaker.

Yeah, that looks like all you would need. And since nsSemanticUnitScanner::Start actually does nothing, and nsSemanticUnitScanner doesn't have any additional member field, you can directly port nsSemanticUnitScanner::Next as a standalone function which takes an nsSampleWordBreaker.

Comment 5

[Jorg K (CEST = GMT+2)]

Had to remove the .h file, too:
https://hg.mozilla.org/comm-central/rev/24b21e9126e0d7e37f44313c7f8fea5c52f972bb

Meanwhile I looked at porting this and it's not so easy since we need nsSampleWordBreaker which is not a published M-C class.

Comment 6

[Xidorn Quan [:xidorn] UTC+11]

I'm not sure what is counted as a published m-c class. It seems to me you can probably get an instance of nsSampleWordBreaker via NS_WBRK_CONTRACTID?

Comment 7

[Jorg K (CEST = GMT+2)]

Third time licky?
https://hg.mozilla.org/comm-central/rev/883c453aed1cc801919dde0ee4a2c54472c645cb

Comment 8

[Jorg K (CEST = GMT+2)]

Attachment: 1368646-move-code.patch

Something like this. Sadly is doesn't compile since GetClass() comes from nsSampleWordBreaker and that's really not available. Or am I missing something?

Comment 9

[Jorg K (CEST = GMT+2)]

Attachment: 1368646-move-code.patch (v2).

OK, this compiles now. I had to copy stuff from nsSampleWordBreaker.cpp since the removed class nsSemanticUnitScanner inherited from nsSampleWordBreaker which doesn't have a public interface, especially the GetClass() method is protected.

Aceman, if you can come up with something nicer, please let me know.

Comment 10

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch for exposing GetClass

I guess this would make your porting easier?

Comment 11

[Xidorn Quan [:xidorn] UTC+11]

Actually it is not clear to me at all why we need the XPCOM things for word breaker... It seems to me they can just be some free functions...

Comment 12

[Jorg K (CEST = GMT+2)]

Comment on attachment 8872877 [details] [diff] [review]
patch for exposing GetClass

Excellent! Many thanks. I'll attach my rebased patch in the next comment.

Comment 13

[Jorg K (CEST = GMT+2)]

Attachment: 1368646-move-code.patch (v3).

Rebased on Xidorn's patch. No more copying of code :-)

Comment 14

[Masatoshi Kimura [:emk]]

Comment on attachment 8872877 [details] [diff] [review]
patch for exposing GetClass

Excellent.

Comment 15

[Jorg K (CEST = GMT+2)]

Comment on attachment 8872891 [details] [diff] [review]
1368646-move-code.patch (v3).

OK, this is good for review based on the M-C patch.

Comment 16

[Jorg K (CEST = GMT+2)]

Xidorn, can you please get the M-C part landed.

Comment 17

[Xidorn Quan [:xidorn] UTC+11]

Not possible at this moment. All trees are closed... I can probably try to land it tomorrow if trees open again.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e8564ce92d8
Expose nsSampleWordBreaker::GetClass as a public static function in nsIWordBreaker. r=emk

Comment 19

[Xidorn Quan [:xidorn] UTC+11]

Ok, trees are reopened now. Landed.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4e8564ce92d8

Comment 21

[:aceman]

Comment on attachment 8872891 [details] [diff] [review]
1368646-move-code.patch (v3).

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

Thanks.

::: mailnews/extensions/bayesian-spam-filter/src/nsBayesianFilter.h
@@ +15,5 @@
>  #include "nsString.h"
>  #include "nsWeakReference.h"
>  #include "nsIObserver.h"
> +#include "nsIWordBreaker.h"
> +#include "nsLWBrkCIID.h"

The CIID file does not seem needed here. Move it to the .cpp file.

Comment 22

[Jorg K (CEST = GMT+2)]

Attachment: 1368646-move-code.patch (v3b).

Moved include and fixed comments.

Comment 23

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/c823bbbed4285550ff7c86a8dc1b05f0a352fdc9