Conversation
|
|
||
| func (d document) ExecCommand(name string, showDefaultUI bool, valueArgument string) bool { | ||
| return d.Call("execCommand", name, showDefaultUI, valueArgument).Bool() | ||
| } |
There was a problem hiding this comment.
valueArgument is documented at https://developer.mozilla.org/en-US/docs/Web/API/Document/execCommand as:
aValueArgument
For commands which require an input argument (such as insertImage, for which this is the URL of the image to insert), this is a DOMString providing that information. Specify null if no argument is needed.
So I don't think we can use string type for it, because it wouldn't be possible to specify null.
We can use either *string, or interface{} and document that it needs to be a string or nil.
@dominikh, do you have thoughts on this?
There was a problem hiding this comment.
All I can tell is this use case worked for me:
dom.GetWindow().Document().ExecCommand("copy", false, "") // document.execCommand('copy');
There was a problem hiding this comment.
I'm not surprised. Most JavaScript APIs will "work" when given input other than their documentation says they expect.
But I still think we should follow the documentation precisely here. Otherwise, people who look at this Go API will have questions raised in their minds, like "does this actually work despite not matching what the JS API expects?" and "will there be unintended side-effects if I don't follow the JS API documentation precisely?"
There was a problem hiding this comment.
Will "" ever be a valid argument? In a lot of other places, it is not, so we special-case "" in the code to act as null. See for example window.GetComputedStyle. – I'd rather not see us use *string or interface{}.
There was a problem hiding this comment.
I don't expect blank string to be a valid argument, but I haven't formally verified.
If there's already precedent for using "" when it's not a valid value to indicate something else (null), then that should be okay.
However, after briefly looking, it appears blank string may be a valid argument for insertText. It's not necessarily a no-op either, because it deletes selection. Also insertHTML.
Is the reason to push back against *string because it's a hard to use API?
In that case, how about *ExecCommandArgument, where ExecCommandArgument is a struct{ Value string }.
Or we can add logic to treat that parameter differently based on command name.
Attempt to fix #37