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

add web app manifest parsing gatherer #48

Merged
merged 7 commits into from
Mar 22, 2016
Merged

add web app manifest parsing gatherer #48

merged 7 commits into from
Mar 22, 2016

Conversation

brendankenny
Copy link
Member

@paullewis

  • some changes were made to the parser to retain more information through the manifest parsing. Each node in the manifest tree retains its original raw value, the parsed value, and the "developer warnings" the spec specifies when you make a mistake, even when recovered. The spec is very forgiving, though, so I've tweaked some things to make sure they still work even while issuing a warning. The original linear _logs is dropped in favor of the warning being associated with the erroneous manifest entry.
  • changes from above mean that at every item in the manifest you have to hit .value to get the parsed value of that node. Kind of annoying, open to ideas there. See changes to test for example.
  • ported tests from https://github.com/mounirlamouri/manifest-validator/blob/master/test/manifest-parser-tests.js. They are very preliminary.
  • added gatherer that executes js in the page context to have the page pull in the manifest itself while we figure out artifact: parsedManifest #22
  • nothing is actually audited yet. Value of parsed manifest is on data.parsedManifest inside your friendly neighborhood auditor

}

// TODO(bckenny): validate color, but for reals
// possibly pull in devtools/front_end/common/Color.js
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as stated here, planning on bringing in e.g. devtools frontend color parsing code rather than dragging in all of jsdom just to do some color parsing code

}

var JSGatherer = {
run: function(driver, url) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in theory this gatherer could collect all the functions that need to run in the context of the page itself (e.g. the one in viewport-meta-tag/gather.js), but long term (as we think about extensibility) it may be too hard to guarantee that these scripts don't step on each other

@@ -0,0 +1,329 @@
/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move from helpers => parsers

I can see the need in the future for multiple parsers

@paullewis paullewis merged commit 432323b into GoogleChrome:master Mar 22, 2016
@paullewis paullewis mentioned this pull request Mar 22, 2016
@brendankenny brendankenny deleted the manifest branch March 22, 2016 18:41
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.

None yet

5 participants