330525 - Can get at enums from nsIDocumentEncoder.idl

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[[e-mail]]

The comment in editorUtilities.js is no longer correct.
Note that the constants were actually inlined (with comments) in bug 88208.

Comment 1

[Serge Gautherie (:sgautherie)]

Neil emailed me
{{
Basically this file

http://lxr.mozilla.org/seamonkey/source/content/base/public/nsIDocumentEncoder.idl

has recently been created, so that JavaScript can now get at the values using (for example) Components.interfaces.nsIDocumentEncoder.OutputWrap rather than writing in the value 32. Originally when the editor code was written all the values were declared as const (but using literal decimal values, while the new file uses shift operators to automatically calculate powers of 2) near line 60 of editorUtilities.js and then the named constants were used. For some reason those constants were removed and replaced by the actual values (but I can't find the patch in the bug mentioned). Obviously we don't want to use the full names everywhere but we can simply declare some constants so we can just use short names (i.e. OutputWrap). Fortunately the original constant names were commented so it should be straightforward to build up a list of the necessary shorthand constants and thus replace the commented constants with the short names.
}}

Comment 2

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) </editor/*.js>

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20060324 SeaMonkey/1.5a] (nightly) (W98SE)

These are the Core </editor> files;
I leave Daniel's Nvu </composer> files for a later patch [or should I leave them alone ?].

Comment 3

[Serge Gautherie (:sgautherie)]

Neil: (nits)
I'm not too sure why you split the |if| clause at line 156 in v1.4 of <EdButtonProps.js>...

Comment 4

[[e-mail]]

Comment on attachment 216216 [details] [diff] [review]
(Av1) </editor/*.js>

You were confused by some incorrect comments but in fact the output flags should always be combined with the | (or |=) operator.

Comment 5

[[e-mail]]

(In reply to comment #2)
>I leave Daniel's Nvu </composer> files for a later patch [or should I leave
>them alone ?].
Best wait as hopefully he will update mozilla/composer from the latest Nvu.

Comment 6

[Serge Gautherie (:sgautherie)]

Attachment: (Av2) </editor/*.js>

Av1, with comment 4 suggestion(s).

(Right: '&' to test, '|' to add :->)

Comment 7

[[e-mail]]

Comment on attachment 216237 [details] [diff] [review]
(Av2) </editor/*.js>

I spotted a (256 = Output encoded entities) in ComposerCommands.js and there are also a few uncommented numbers in EditorCommandsDebug.js which look as if they should be converted to use the new constants.

>+    selection = editor.outputToString("text/html", kOutputFormatted | kOutputSelectionOnly | OutputWrap);
kOutputWrap, surely?

Comment 8

[Serge Gautherie (:sgautherie)]

Attachment: (Av3) </editor/*.js> [Checked in: Comment 12]

Av3, with comment 7 suggestion(s).

(For Av2, I had searched LXR only for the const from the idl file;
For Av3, I searched MXR for |flags |=, outputToString, toStringWithFormat|.)

NB: In <EditorCommandsDebug.js>, I wonder if/why the two values at lines 103+ should really be the same for 'text/plain' and 'text/html' !?

Comment 9

[[e-mail]]

(In reply to comment #8)
>NB: In <EditorCommandsDebug.js>, I wonder if/why the two values at lines 103+
>should really be the same for 'text/plain' and 'text/html' !?
I don't see what's wrong with them using the same flags with different formats.

Comment 10

[Serge Gautherie (:sgautherie)]

(In reply to comment #9)
> (In reply to comment #8)
> >NB: In <EditorCommandsDebug.js>, I wonder if/why the two values at lines 103+
> >should really be the same for 'text/plain' and 'text/html' !?
> I don't see what's wrong with them using the same flags with different formats.

I don't say that there is anything wrong.
I simply wondered what the point was, and if some 'kOutputEncodeW3CEntities' or whatever would be helpful ... in case the current situation was some kind of copy and paste leftover...
If you say that this is intended, fine with me.

Comment 11

[[e-mail]]

Comment on attachment 216477 [details] [diff] [review]
(Av3) </editor/*.js>
[Checked in: Comment 12]

glazou also said "YES-YES-YES !!!" over IRC without even looking at the patch ;-)

Comment 12

[[e-mail]]

Fix checked in, thanks :-)

Comment 13

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1) <EdButtonProps.js> nit

(See comment 3)

Comment 14

[Philip Chee]

Comment on attachment 216666 [details] [diff] [review]
(Bv1) <EdButtonProps.js> nit

Removing review requests due to severe bit rot. Please file a new bug for this remaining issue.