74927 - Need a generic way to pass arguments to commands

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect, P3)

Description

[Simon Fraser [no longer active]]

We need a generic way to pass arbitrarily complex sets of parameters to commands, 
for the following reason:

We've decided that it's a Good Thing to provide to embedders a simple way to 
'execute' user-level commands ("Make bold", "Insert table"), via a simple 
DoCommand/IsCommandEnabled interface. However, many such commands need to bring 
up UI to get further input from the user (e.g. rows and columns for table 
insertion). Unless we provide some way to package up command paramters, embedders 
would not be able to use the simple DoCommand API to execute such commands. This 
would force them to use different APIs for different types of commands, which 
would suck.

So nsICommandParams provides a way to package up a set of parameters as name-
value pairs, where values can be of a number of primitive types.

Of course, as well as documenting what commands are supported by a component, 
we'll also need to document the require and optional paramters that those 
commands require.

We can also use nsICommandParams to allow the embedder to communicate to us what 
kinds of command state it cares about (embedder hands us an nsICommandParams with 
entries for the values it cares about [e.g. "enabled", "bold"] and we can fill in 
the values and return it.

Comment 1

[Simon Fraser [no longer active]]

Attachment: nsICommandParams.idl

Comment 2

[Simon Fraser [no longer active]]

Attachment: nsCommandParams.cpp

Comment 3

[Simon Fraser [no longer active]]

Attachment: nsCommandParams.h

Comment 4

[Simon Fraser [no longer active]]

This stuff lives in mozilla/embedding/components/commandhandler.

Looking for r=, sr=

I'll be adding an nsICommandManager interface that takes this command params 
soon.

Comment 5

[Adam Lock]

A couple of comments:

o  No comments in IDL
o  Why is DOMString being used in the IDL for the name of arguments and string 
values? Shouldn't we use string for names and wstring for values?
o  nsCommandParams::GetLongValue sets _retval to PR_FALSE, should be 0

Comment 6

[rubydoo123]

since it is getting reviewed now, setting to moz0.9

Comment 7

[Simon Fraser [no longer active]]

> o  No comments in IDL

I have a more recent version of the file with comments

> o  Why is DOMString being used in the IDL for the name of arguments and string 
> values? Shouldn't we use string for names and wstring for values?

DOMString (which provides nsAReadable/nsAWritable strings in the C++ headers) is 
used because it allows for more efficient string usage, and avoidance of excess 
copying and allocation, yet can be used from JS just like string/wstring.

As to whether value names should be ASCII or Unicode, I don't see a strong 
argument either way. Keeping everything as Unicode simplifies the interface, but 
I can see the value of using narrow strings.

> o  nsCommandParams::GetLongValue sets _retval to PR_FALSE, should be 0

Oops, thanks for that one.

Comment 8

[kinmoz]

nsICommandParams.idl should probably use nsAString instead of nsDOMString since 
this isn't really related to the DOM. The XPIDL compiler will generate the same 
nsAReadableString/nsAWritableString types so you shouldn't have to change 
anything in your implementation.

Comment 9

[Simon Fraser [no longer active]]

-> 0.9.1

Comment 10

[Michael Dunn]

Correction: Changing QA contact for the Embed API bugs to David Epstein.

Comment 11

[rubydoo123]

1.0

Comment 12

[Asa Dotzler [:asa]]

Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)

Comment 13

[Simon Fraser [no longer active]]

->mjudge

Comment 14

[Simon Fraser [no longer active]]

This landed with the editor embedding stuff.

Comment 15

[David Epstein]

verifying against Mozilla 0.9.9 3/22/02:
* nsICommandParams.idl. verifying change from DOMString to AString type.
* nsICommandParams.cpp. verifying change from DOMString to AString, also
nsAReadableString to nsAString type in various methods.