790341 - /delayed command loses execution context for command

Status: RESOLVED FIXED

(Other Applications Graveyard :: ChatZilla, defect)

Description

[James Ross]

When a command is executed, it has a context (stored in "e"), which contains the sourceObject and related data about where it was run. Many commands operate on this. The /delayed command does not preserve this when executing its delayed command argument, meaning that the current view is used instead of the one the /delayed command was executed on. This can be especially bad for auto-perform /delayed commands.

Comment 1

[Donald Sarratt]

Attachment: Make delayed commands execute in the proper view.

Passing the context to the dispatch() call seems to fix this, for both auto-performed and manually-entered delays. I don't think there's a situation where the context could be deleted between the cmdDelayed() call and the dispatch() call, but if it does get deleted we just have a reversion to previous behaviour (using the current view).

Comment 2

[James Ross]

Comment on attachment 687468 [details] [diff] [review]
Make delayed commands execute in the proper view.

Review of attachment 687468 [details] [diff] [review]:
-----------------------------------------------------------------

Happy with this patch if either modification I suggest resolves the issue I believe exists (or you can show the issue doesn't exist).

::: xul/content/commands.js
@@ +1113,4 @@
>  {
>      function _dispatch()
>      {
> +        dispatch(e.rest, e, e.isInteractive);

I believe we need to delete e.inputData before this call, otherwise if you delay an unknown command with no arguments it messes up and sends "/quote command delay command" (i.e. appends the /delayed arguments).

(We could also fix that in dispatch() itself, where it sets e.inputData to ary[2] on line 349, by adding an else which sets it to "".)

Comment 3

[Donald Sarratt]

Attachment: Fix for bug 790341

That bug you mentioned applies to any command with no arguments (e.g. "/delayed 5 leave" will attempt to leave a channel called 5 if run outside a channel view). I wasn't able to track down what's causing this, but the fix you mentioned does work.

Comment 4

[James Ross]

Comment on attachment 692552 [details] [diff] [review]
Fix for bug 790341

Review of attachment 692552 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

The problem was that after splitting command and arguments at http://hg.mozilla.org/chatzilla/file/aef5fb39a211/xul/content/commands.js#l339 we only set e.inputData if it was split, not when it is just a command (which is what we have in this patch).

Comment 5

[James Ross]

http://hg.mozilla.org/chatzilla/rev/c0e8aca3cf41