65677 - pref UI: if no spellchecker installed, pref should be disabled

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[sairuh (rarely reading bugmail)]

pls pardon if this is a dup --couldn't find an existing bug on this. occurs on
all platforms.

0. make sure your installation doesn't include the spellchecker --ie, the
components/spellchecker/ dir should not exist.
1. start app, open Preferences dialog.
2. open the Mail and Newsgroups > Message Composition panel.

observe: under the Composing Messages section, the "Check spelling before
sending is enabled --ie, you can select and deselect the checkbox.

expected: if there's no spellchecker, this feature should greyed out [disabled].

Comment 1

[Gervase Markham [:gerv]]

If someone said how to check whether the spellchecker was installed or not, I'm 
sure someone could insert the logic into the JS for this without too much 
trouble...

Gerv

Comment 2

[(not reading, please use [e-mail] instead)]

gerv,

in nsMessengerMigrator.cpp I have code that sees if the 4.x addressbook upgrader
component is installed.

in C++, it's something like this:

nsCOMPtr <nsIAbUpgrader> abUpgrader = do_GetService(NS_AB4xUPGRADER_CONTRACTID,
&rv);
if (NS_FAILED(rv) || !abUpgrader) return;

you could do the same thing from JS for spell checker.

(see what they are doing in nsEditorShell::InitSpellChecker())

Comment 3

[Gervase Markham [:gerv]]

Seth, I have no idea how to express that idea in JS :-(

In all seriousness, surely this should just be disabled in Mozilla? If Netscape 
want to add a spell checker, they'll have to re-enable it in their builds. ;-)

For reference:
http://lxr.mozilla.org/seamonkey/source/mailnews/compose/prefs/resources/content
/pref-messages.xul#66
is the location of the XUL for the offending item.

(This file does not have an associated .js file so we'd have to create one to 
contain the logic if we greyed it out dynamically.)

Gerv

Comment 4

[Gervase Markham [:gerv]]

OK - patch coming up :-) It's five lines, and it really _should_ work, but I 
can't test it properly because I don't have access to a build with spellchecker 
installed :-)

Could I get some review, please? :-)

Gerv

Comment 5

[Gervase Markham [:gerv]]

Attachment: Patch

Comment 6

[Gervase Markham [:gerv]]

Attachment: Better patch (r=blake)

Comment 7

[(not reading, please use [e-mail] instead)]

looking good.

two comments:

before setting disabled to false, I think you should QI to see that what you got 
really is an nsISpellChecker.

did you need help testing this code, to see if we do the right thing, when a 
spell checker is installed?

Comment 8

[Gervase Markham [:gerv]]

> before setting disabled to false, I think you should QI to see that what 
> you got really is an nsISpellChecker.

Can't I trust the call to return an nsISpellChecker or null? <shrug>. Could you
provide a sample line or a doc? I don't know much about QueryInterface...

> did you need help testing this code, to see if we do the right thing, 
> when a spell checker is installed?

Indeed. I cribbed the code from somewhere in Mail/News, so it really should
work, but whoever checks it in should probably test it first :-)

Gerv

Comment 9

[(not reading, please use [e-mail] instead)]

here's how I'd alter your patch

var spellChecker;
try {
	spellChecker = Components.classes["@mozilla.org/
spellchecker;1"].QueryInterface(Components.interfaces.nsISpellChecker);
}
catch (ex) {
	spellChecker = null;
}

if (spellChecker) {
	// enable the UI
}

can you create a new patch, like that?

then I'll try it out my tree (with the spell checker built and installed, and 
without)

thanks work doing this, gerv.  do you want to re-assign the bug to you?

Comment 10

[Gervase Markham [:gerv]]

Attachment: 3rd attempt

Comment 11

[Gervase Markham [:gerv]]

Attempt 3 attached, incorporating comments from sspitzer. Give it a go :-)

Gerv

Comment 12

[(not reading, please use [e-mail] instead)]

update:  sorry gerv, didn't get to it today.  I'll do it tomorrow (3/28)

Comment 13

[(not reading, please use [e-mail] instead)]

Attachment: the final fix

Comment 14

[(not reading, please use [e-mail] instead)]

note, I couldn't do a create instance, since nsISpellChecker is not scriptable.

but gerv's original way worked fine for now.  I've tested and it all works.

ideally, nsISpellChecker should be made scriptable.  I'll log another bug for
that.

thanks again gerv.

Comment 15

[Gervase Markham [:gerv]]

If you do that, and re-fix this, you'll also need to update the code in the 
place I nicked it from - I think it was the bit that enabled or disabled the 
SpellCheck button.

Gerv

Comment 16

[esther]

verified because the pref is no longer visible when a spell checker is not
installed see bug 84607, verified on 3-29-2002.