84607 - Hide spellchecker pref

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect)

Description

[Ben Bucksch (:BenB)]

If the spellchecker is not installed, the pref checkbox is disabled. It should
be hidden. (Ideally not exist at all, but that's another thing.)

Comment 1

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

I remember reviewing the patch to make it so the UI was disabled when the spell 
checker wasn't installed.

see bug #65677

see 
http://lxr.mozilla.org/seamonkey/source/mailnews/compose/prefs/resources/content
/pref-composing_messages.xul#51

Comment 2

[Sheela Ravindran]

change qa contact->esther 

Comment 3

[Mark Stosberg]

I think if the spell checker is not installed, the UI should let you know how to
install it. I was about to report the missing spell-checker as a bug! I think
it's good to let the user know the possibility of a spell-checker exists when
it's missing. 

Comment 4

[Ben Bucksch (:BenB)]

Just that none exists, at least no open-source one.

Comment 5

[Boris Zbarsky [:bzbarsky]]

*** Bug 88937 has been marked as a duplicate of this bug. ***

Comment 6

[Ben Bucksch (:BenB)]

Attaching fix.

Seth, please review.

Comment 7

[Ben Bucksch (:BenB)]

Attachment: Fix, version 1.

Comment 8

[Håkan Waara]

Makes no sense to have it disabled if it's not possible to enable it (at least
not for Mozilla).

r=hwaara

Comment 9

[timeless]

*shrug* i'm working on getting amaya's spellchecker working in moz, it'll be 
harder for me to find the thing to enable if i can't see it.

Comment 10

[Ben Bucksch (:BenB)]

timeless, check the patch to see where the code is :). I believe that this is
the right thing to do, even if we have an open-source spellchecker. We don't
have UI for Mailnews either, if it isn't installed.

Comment 11

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

sr=sspitzer

please make sure to that it still shows up when there is a spell checker.

Comment 12

[lohphat]

What's the status on an open-source spellchecker?  I don't have one installed,
but the pref is still showing up selected, but greyed.

Comment 13

[lohphat]

Attachment: Screenshot of behavior

Comment 14

[Ben Bucksch (:BenB)]

ahhhh

Comment 15

[timeless]

1. nothing in this bug indicates benb committed the patch
2. i'm currently employed in other endeavors and my spellchecker impl is on my
home pc collecting dust.  Afaik there is no currently active open source
activity on a spell checker.

Comment 16

[Chris P. Ross]

I think I'm in disagreement with this fix.  Dunno if anyone else cares
what I think, but.  I think it's important to let people know that mozilla
knows how to do spell checking, even if not internally.  When I wanted to
know if mozilla could do it, I looked at the prefs.  When I saw the disabled
pref, I looked into why it was disabled, rather than just throwing my hands
up and thinking "mozilla sucks that it doesn't know how to do that".

AFA open-source spell checkers, what about ispell?  Or aspell/pspell 
(http://aspell.sourceforge.net/)  Perhaps we could just get the aspell
library and just put it into mozilla?  Or at least make it know how to
use it internally via linking against that library...

Comment 17

[Gervase Markham [:gerv]]

> AFA open-source spell checkers, what about ispell?  Or aspell/pspell 

http://www.mozilla.org.uk/docs/spell-checker-faq.html .

Gerv

Comment 18

[gavin long]

*** Bug 114280 has been marked as a duplicate of this bug. ***

Comment 19

[eberry]

This (simple but effective) patch has an r and sr, but hasn't been checked in.
If anyone knows the reason it hasn't been checked in please add it in a comment 
or a status whiteboard update.  Is it because some future Netscape spell 
checker xpi might work with the moz 0.9.7 release and this patch would still 
hide the pref?

Comment 20

[Gervase Markham [:gerv]]

It hasn't been checked in because no-one's got around to it. BenB?

Gerv

Comment 21

[Ben Bucksch (:BenB)]

My fault. Was not high on my priv list.

Seth, can I still check this in 0.9.8 (because of the perf etc. focus)? (I will
make sure that the stuff still works.)

Comment 22

[Jerome Lacoste]

status for the patch attachment should perhaps be updated. (r= and sr=?)
patch should perhaps be checked in if it has not rotted.

Comment 23

[Ben Bucksch (:BenB)]

Comment on attachment 41261 [details] [diff] [review]
Fix, version 1.

It has been rotted, but it is easy to adapt. Not sure, if I need new reviews
for that.

Comment 24

[Håkan Waara]

If the code change is exactly the same, you don't need new review.

Comment 25

[Jean-Francois Ducarroz]

cc'ing varada for review...

Comment 26

[Ben Bucksch (:BenB)]

Attachment: Fix, version 2

It got even more trivial...

Sorry for the checkin delay. Will check in at least with my next checkin.

Comment 27

[Daniel Brooks [:db48x]]

Comment on attachment 74356 [details] [diff] [review]
Fix, version 2

r=db48x

Comment 28

[Jean-Francois Ducarroz]

R=ducarroz (as module owner)

Comment 29

[David :Bienvenu]

Comment on attachment 74356 [details] [diff] [review]
Fix, version 2

sr=bienvenu

Comment 30

[Asa Dotzler [:asa]]

Comment on attachment 74356 [details] [diff] [review]
Fix, version 2

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 31

[Ben Bucksch (:BenB)]

Checked in. Sorry that it took so long. Thanks for the reviews/a.

I had no way to test it with the NS spellchecker installed. But the fix is
trivial enough :).

Comment 32

[esther]

Using build 20020328 on winxp and linux this is fixed.  However on Mac, without
a spell checker installed I still see this preference and it's not disabled,
also the Spell button is on the toolbar and in the options menu in an enabled
state. Clicking on any of these result in nothing happening.   Reopening for the
Mac OS9.1, if I should write a new bug let me know

Comment 33

[Ben Bucksch (:BenB)]

This sounds like unrelated local horkage to me, esp. since I haven't touched the
Composer toolbar button. Doesn't the mac have some central registry? Make sure
that there are no trails of an old / other spellchecker installation accessible.

Comment 34

[Asa Dotzler [:asa]]

ben, can you resolve this or mark the landed patch as obsolete so this doesn't
look like an approved patch waiting to land? Thanks.

Comment 35

[Ben Bucksch (:BenB)]

o-kay...
I believe this is fixed. See my last comment.

Comment 36

[esther]

I will check the mac with a clean installation and update.

Comment 37

[esther]

I'll ask Grace if she can check the Mac on a clean install. 

Comment 38

[esther]

Grace ran the test on a Mac with no previous mozilla app.  However Communicator
4.7x was installed and may have had a spellchecker installed.  When installing
the latest build without spell check she still saw the spell button and
preference but they did nothing.  If Ben's statement is true that Mac may have a
central registry and any app with a spellchecker will cause these toolbar
buttons and preferences to display then I can't test this on the Mac and will
verify for win and linux.  This may not be an issue for Mac users if most of the
apps they install have spellcheckers.  

Comment 39

[Ben Bucksch (:BenB)]

> When installing the latest build without spell check she still saw the spell
> button and preference but they did nothing.

Most likely, this was true even before this change. Thanks for vrfying.