1118825 - None value in script_args for execute_script() should not raise an exception

Status: VERIFIED FIXED

(Remote Protocol :: Marionette, defect, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1] (away from Feb 12th - Feb 22nd)]

The following snippet causes an exception to be raised in Marionette:

        retval = self.marionette.execute_script("""
            return arguments[1];
        """, script_args=['test', None])
        assert retval is None

Exception:

Traceback (most recent call last):
  File "/mozilla/code/firefox/inbound/testing/marionette/client/marionette/marionette_test.py", line 268, in run
    testMethod()
  File "/mozilla/code/marionette-tests/greenlight/lib/tests/test_prefs.py", line 36, in test_get_pref
    """, script_args=['test', None])
  File "/mozilla/code/firefox/inbound/testing/marionette/client/marionette/marionette.py", line 1282, in execute_script
    filename=os.path.basename(frame[0]))
  File "/mozilla/code/firefox/inbound/testing/marionette/client/marionette/decorators.py", line 36, in _
    return func(*args, **kwargs)
  File "/mozilla/code/firefox/inbound/testing/marionette/client/marionette/marionette.py", line 634, in _send_message
    self._handle_error(response)
  File "/mozilla/code/firefox/inbound/testing/marionette/client/marionette/marionette.py", line 663, in _handle_error
    "Malformed packet, expected key 'error' to be a dict: %s" % response)
MarionetteException: MarionetteException: Malformed packet, expected key 'error' to be a dict: {u'message': u'arg is null', u'error': u"error occurred while processing 'executeScript"}

Instead None should become undefined in Javascript, and returned as None again.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

I would expect None to map to null. I think this function just needs a null check:
https://hg.mozilla.org/mozilla-central/file/7507dfe42c11/testing/marionette/marionette-elements.js#l384

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1] (away from Feb 12th - Feb 22nd)]

Ah, you are right. null is indeed the better value here.

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Actually, applyNamedArgs functionality doesn't look like it's used anywhere. :jgriffin, do you happen to know if this is used or what we should do with it?

Comment 4

[Jonathan Griffin (:jgriffin)]

applyNamedArgs is used to pass element references between calls on the Python side.  So you can do something like:

button = marionette.find_element('id', 'foo')
print button
>>> # some uuid
marionette.execute_script("arguments[0].click();", script_args=button)

Comment 5

[Jonathan Griffin (:jgriffin)]

(In reply to Jonathan Griffin (:jgriffin) from comment #4)
> applyNamedArgs is used to pass element references between calls on the
> Python side.  So you can do something like:
> 
> button = marionette.find_element('id', 'foo')
> print button
> >>> # some uuid
> marionette.execute_script("arguments[0].click();", script_args=button)

...and I realize that doesn't answer your question.  I think we don't use applyNamedArgs any longer; we used to use it in some idiosyncratic B2G frameworks that don't exist any longer, as in:

https://hg.mozilla.org/mozilla-central/annotate/e934a9d8be1f/testing/marionette/client/marionette/marionette_test.py#l165

Comment 6

[Chris Manchester (limited bugmail, email directly)]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=5cd8a4930a43

Comment 7

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: bz://1118825/chmanchester

Comment 8

[Chris Manchester (limited bugmail, email directly)]

/r/2205 - Bug 1118825 - Allow passing None as an argument to executeScript in chrome scope.;r=ato

Pull down this commit:

hg pull review -r 83cb0b0c1a631eccec9559d5fc8b088f279d24a0

Comment 9

[Chris Manchester (limited bugmail, email directly)]

/r/2205 - Bug 1118825 - Allow passing None as an argument to executeScript in chrome scope.;r=ato

Pull down this commit:

hg pull review -r 83cb0b0c1a631eccec9559d5fc8b088f279d24a0

Comment 10

[Jonathan Griffin (:jgriffin)]

https://reviewboard.mozilla.org/r/2203/#review1651

Ship It!

Comment 11

[Chris Manchester (limited bugmail, email directly)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7359afa42350

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7359afa42350

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1] (away from Feb 12th - Feb 22nd)]

Works fine. Thanks!

Comment 15

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1118825 - Allow passing None as an argument to executeScript in chrome scope.;r=ato