Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#14592 closed defect (fixed)

Parser does not handle args parameter properly

Reported by: Kitson Kelly Owned by: bill
Priority: high Milestone: 1.8
Component: Parser Version: 1.7.1
Keywords: Cc:
Blocked By: Blocking:

Description

When the parser is invoked, and only the args is passed, but it does not contain a rootNode, the parser will not set the args, which is not the expected behaviour. The following example works:

parser.parse({
  mixin: myMixin,
  rootNode: myNode
});

But the following would not:

parser.parse({
  mixin: myMixin
});

The attached patch resolves this issue.

Attachments (3)

parser_args.js.patch (424 bytes) - added by Kitson Kelly 7 years ago.
Correctly handles args without rootNode
parser-args.html (2.3 KB) - added by Kitson Kelly 7 years ago.
A unit test showing the defect with args without rootNode
Fixes-14592--Handling-of-arguments-for-parserparse.patch (373 bytes) - added by Kitson Kelly 7 years ago.
Updated patch for TRUNK parser

Download all attachments as: .zip

Change History (11)

Changed 7 years ago by Kitson Kelly

Attachment: parser_args.js.patch added

Correctly handles args without rootNode

comment:1 Changed 7 years ago by Kitson Kelly

Sorry wrong ticket...

Last edited 7 years ago by Kitson Kelly (previous) (diff)

comment:2 Changed 7 years ago by bill

OK, not sure exactly what you mean by "set the args", but how about adding some tests into your patch file after the checkin to #13778 is finished?

Last edited 7 years ago by bill (previous) (diff)

comment:3 Changed 7 years ago by Kitson Kelly

Following up on this one, the main reason the current unit test doesn't highlight this is because it always supplies a rootNode to the parser.parse() and this only manifests itself when the rootNode is not passed a parameter and not passed in the args and there is no easy way to test for it within the current unit test because none of them parse the whole document.

So I am attaching a stripped down version of the unit test case that demonstrates the issue. It essentially does the following:

parser.parse({
  scope: "myscope"
});

Which, without this pass fails to change the scope and therefore doesn't parse the object. With the patch, this test passes fine. I think it hasn't been discovered up to this point because very few people would be parsing the whole document but require a set of args, but when I was working on #13779 and trying to pass the context in the args, I discovered it doesn't work, although there is an example in the inline documentation that indicates args without rootNode should be supported.

Changed 7 years ago by Kitson Kelly

Attachment: parser-args.html added

A unit test showing the defect with args without rootNode

Changed 7 years ago by Kitson Kelly

Updated patch for TRUNK parser

comment:4 Changed 7 years ago by Kitson Kelly

Updated the patch to match TRUNK parser.

comment:5 Changed 7 years ago by bill

Milestone: tbd1.8
Status: newassigned

comment:6 Changed 7 years ago by bill

Resolution: fixed
Status: assignedclosed

In [27670]:

Fix variable argument support in parse() function, and create separate directory for parser tests, fixes #14592 !strict.

comment:7 Changed 7 years ago by bill

In [27697]:

Turns out the extra check was needed, otherwise themeTester.html was failing on IE, refs #14592 !strict.

comment:8 Changed 7 years ago by bill

In [29414]:

use relative paths in require.toUrl(), rather than depending on weird mapping from "tests" to "dojo/tests", fixes parser tests in built version of dojo, refs #14592

Note: See TracTickets for help on using tickets.