Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

URL Safe #38

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

URL Safe #38

wants to merge 2 commits into from

Conversation

lavnrose
Copy link

@lavnrose lavnrose commented Apr 6, 2016

Usually URL can contain special character like '&', and it must use urlencode.
For easy to use, we can use CDATA.

@tbuchok
Copy link
Owner

tbuchok commented Apr 12, 2016

@lavnrose looking at this momentarily -- needed to get full xcode on my machine as libxml i think needed the full version and not just command line. hadn't rebuilt this repo since changing computers a few months ago!

@tbuchok
Copy link
Owner

tbuchok commented Apr 12, 2016

@lavnrose this doesn't look to pass the xsd schema validation. did you experience different results:

$ npm t

yields en error:

# validates non-linear vast xml
not ok 74 It validates against the VAST .xsd
  ---
    file:   /Users/tombuchok/vast-xml/test/index.test.js
    line:   39
    column: 5
    stack:  
      - getCaller (/Users/tombuchok/vast-xml/node_modules/tap/lib/tap-assert.js:418:17)
      - Function.assert (/Users/tombuchok/vast-xml/node_modules/tap/lib/tap-assert.js:21:16)
      - Test._testAssert [as ok] (/Users/tombuchok/vast-xml/node_modules/tap/lib/tap-test.js:86:16)
      - Test.<anonymous> (/Users/tombuchok/vast-xml/test/index.test.js:39:5)
      - Test.emit (events.js:129:20)
      - Test.emit (/Users/tombuchok/vast-xml/node_modules/tap/lib/tap-test.js:103:8)
      - GlobalHarness.Harness.process (/Users/tombuchok/vast-xml/node_modules/tap/lib/tap-harness.js:86:13)
      - process._tickCallback (node.js:355:11)
      - Function.Module.runMain (module.js:503:11)
      - startup (node.js:129:16)
  ...

i'd prefer not to merge this until we resolve the error. can you take a look?

@lavnrose
Copy link
Author

@tbuchok I am sorry to confuse you. the issue have been fixed now, also I've checked it as below.

# validates non-linear vast xml
ok 74 It validates against the VAST .xsd
# validates wrapper vast XML
ok 75 It validates against the VAST .xsd
# validates wrapper with companion vast XML
ok 76 it ensures that companion ads are present
ok 77 It validates against the VAST .xsd
# omit tracking
ok 78 It should not include impressions
# validates vast with top level error tag and no ads
ok 79 It has Error element
ok 80 It has not Ad element

1..80
# tests 80
# pass  80

# ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants