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

VERIFIED FIXED in mozilla38

Status

defect
P1
normal
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: whimboo, Assigned: chmanchester)

Tracking

({pi-marionette-server})

unspecified
mozilla38
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [marionette=1.0])

Attachments

(1 attachment, 1 obsolete attachment)

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.
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
Ah, you are right. null is indeed the better value here.
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?
Flags: needinfo?(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)
Flags: needinfo?(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
Attachment #8546001 - Flags: review?(ato)
/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
Priority: -- → P1
Whiteboard: [marionette=1.0]
Assignee

Updated

4 years ago
Assignee: nobody → cmanchester
Assignee

Updated

4 years ago
Attachment #8546001 - Flags: review?(jgriffin)
/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
Attachment #8546001 - Flags: review?(jgriffin) → review+
Assignee

Updated

4 years ago
Attachment #8546001 - Flags: review?(ato)
https://hg.mozilla.org/mozilla-central/rev/7359afa42350
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Works fine. Thanks!
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
Attachment #8546001 - Attachment is obsolete: true
Attachment #8619056 - Flags: review+
You need to log in before you can comment on or make changes to this bug.