67993 - [embed]Using plain text email causes JS errors because of unnecessary commands in editor

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Christopher Blizzard (:blizzard)]

Build is 2001 02 07 11.  When replying to a piece of email I'm getting this as
an error ( but the reply still seems to succeed. )

failed to get command manager number 3
failed to get command manager number 2
Registering commands
Have Find = true
Have SpellChecker = false
RECEIVE CALLBACK: OnHeadersReady
replacing child in comp fields 2 recips 
Warning prev sibling is not in our list!!!set focus on the body
************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x80004001
(NS_ERROR_NOT_IMPLEMENTED) [nsIEditorShell.GetSelectedElement]"  nsresult:
"0x80004001 (NS_ERROR_NOT_IMPLEMENTED)"  location: "JS frame ::
chrome://editor/content/editor.js :: GetObjectForProperties :: line 970"  data: no]
************************************************************
An error occurred updating the cmd_objectProperties command

Comment 1

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

this is a duplicate.  over to ducarroz.

blizzard, this is plain text reply, right?  It shouldn't actually cause you any
problems.

Comment 2

[Christopher Blizzard (:blizzard)]

Yeah, this is in plain text mail.  It doesn't seem to hurt anything that I can
see but a JS error is a JS error.  If you happen to know that it isn't breaking
anything then it's not a big deal.  It would be nice to get it cleaned up for
0.8 though.  What do you think?

Comment 3

[Simon Fraser [no longer active]]

cc editor folks. My guess is that cmanske should get this.

Comment 4

[Charles Manske]

Sure, I'll take this. We are probably registering the same commands for both
HTML and plain text mail. The error indicates that a HTML UI command is (trying)
to be updated.
I don't think this should be rushed into 0.8, so setting to 0.9.

Comment 5

[kinmoz]

Are we sure that the error isn't causing some JS code, that *needs* to be
executed, to be skipped? If it is causing code to be skipped, I think we might
need to fix this for 0.8.

Comment 6

[Charles Manske]

I investigated thoroughly and the cause is exactly as I described: We register
the same set of commands for both HTML and plain text mail, even though commands
such as nsEditorShell::GetSelectedElement() are not implemented for plain text.
The error is not critical.
The correct solution is to register only the required commands for the plain
text editor. We are in the midst of rebuilding this as part of embedding
rearchitecture and I'll fix this aspect as part of that work (my part is
covered by bug 66300).

Comment 7

[Charles Manske]

Changing "depends" to our embed-tracking bug.

Comment 8

[Fabian Guisset]

this is a dup of bug 63396 or rather bug 63396 is a dup of this one, since this
one is assigned and more advanced in its resolution.

Comment 9

[Charles Manske]

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

Comment 10

[Charles Manske]

Attachment: Separate registration of HTML and Plain Text commands

Comment 11

[Charles Manske]

The essential change is rather small -- most of the lines touched in the 3/02/01
attachment is because I changed gHTMLEditorCommandManager to
gEditorCommandManager so it wouldn't be missleading.
There's also a small cleanup change at the end of editor.js patch that simply
removes the unused varible "menuList" and does null check on "paraMenuList"
passed in (this is what replaced "menuList" var.)

Comment 12

[Charles Manske]

Attachment: Update on fix: allow Insert Character dialog in text editor

Comment 13

[Kathleen :Brade]

I'd rather see GetEditorController() maintain gEditorCommandManager as opposed to 
having the caller of GetEditorController need to determine whether or not it 
should call GetEditorController.  This would cleanup SetupTextEditorCommands() as 
well as SetupHTMLEditorCommands().

Comment 14

[Sheela Ravindran]

reassing back to esther,

Comment 15

[Charles Manske]

Attachment: Updated fix to cache gEditorCommandManager in GetEditorController()

Comment 16

[Simon Fraser [no longer active]]

Why does gEditorCommandManager have to be global at all? We only ever use it 
inside of the SetupHTMLEditorCommands() function.

Comment 17

[Charles Manske]

Attachment: Eliminate global "controller" variables altogether

Comment 18

[Simon Fraser [no longer active]]

sr=sfraser

Comment 19

[Charles Manske]

checked in.

Comment 20

[Charles Manske]

fixed

Comment 21

[[e-mail] (gone - use [e-mail] instead)]

I don't see this anymore with build 2002-02-04-12, Linux (RedHat 7.2)
Using plaintext reply and the console.

Verified FIXED.