Load html #347

gkellogg · 2019-12-28T01:35:42Z

Load JSON-LD from HTML documents.

Adds options parameter to documentLoader
Uses xmldom, if loaded.
Adds util.ParseContentTypeHeader
Adds documentLoader implementations for xhr and node (still requires tests).
Set default for expandAllScripts to true for flatten and toRdf.

gkellogg · 2019-12-28T01:37:22Z

Some tests still required for XHR and Node documentLoaders.

Some work needed to optionally append text/html (and application/xhtml+xml) to accept headers if we can parser HTML.

Perhaps some abstracted parsing support to handle other HTML parsers?

Otherwise, please see if you agree with the general direction of this work.

gkellogg · 2019-12-28T20:31:00Z

lib/jsonld.js

@@ -865,7 +871,10 @@ jsonld.get = async function(url, options) {
    load = jsonld.documentLoader;
  }

-  const remoteDoc = await load(url);
+  // FIXME: unescape frag?


Not sure if this is necessary; it's not to pass tests.

gkellogg · 2019-12-28T20:31:33Z

lib/jsonld.js

+            continue;
+          }
+          try {
+            remoteDoc.document.push(JSON.parse(script.textContent));


Note that textContent always decodes entities, so there are tests we can't pass. Any ideas?

FWLIW, I threw together a JSBin that uses example 10 to test parsing via jsonld.js 2.0.2:
https://jsbin.com/rewaxiquki/edit?html,console

There aren't any entity decoding issues, so I think this issue is probably a bug in xmldom's implementation.

Here's another one which works using jsdom:
https://runkit.com/embed/9qrv8dl5g2bs

FYI, I use htmlparser2 in my implementation, and it doesn't seem to be decoding entities, so it allows all tests to pass.
So I also think that this is a bug/feature in xmldom.

gkellogg · 2019-12-28T20:32:25Z

lib/jsonld.js

@@ -934,6 +995,20 @@ jsonld.documentLoaders = {};
 jsonld.documentLoaders.node = require('./documentLoaders/node');
 jsonld.documentLoaders.xhr = require('./documentLoaders/xhr');

+// Optional DOM parser
+try {
+  jsonld.domParser = require('xmldom').DOMParser;


This will support xmldom, if it's loaded. Are there other parsers that should be included? How to configure and test?

gkellogg · 2019-12-28T20:33:17Z

lib/util.js

@@ -15,6 +15,7 @@ const REGEX_LINK_HEADER = /\s*<([^>]*?)>\s*(?:;\s*(.*))?/;
 const REGEX_LINK_HEADER_PARAMS =
  /(.*?)=(?:(?:"([^"]*?)")|([^"]*?))\s*(?:(?:;\s*)|$)/g;

+// FIXME: conditinally support text/html


With HTML support, we should include text/html and application/ld+json, but not sure best way to do that. It will also mess up some documentLoader tests?

gkellogg · 2019-12-28T20:34:09Z

lib/documentLoaders/xhr.js

@@ -30,7 +34,7 @@ module.exports = ({
  const queue = new RequestQueue();
  return queue.wrapLoader(loader);

-  async function loader(url) {
+  async function loader(url, options) {


There don't seem to be standalone XHR documentLoader tests, as there are node tests.

gkellogg · 2019-12-28T20:40:18Z

Perhaps the webpack build is failing because it doesn't include xmldom? Should there be a way to condition tests on this? How?

lib/documentLoaders/node.js

lib/documentLoaders/xhr.js

lib/jsonld.js

dlongley · 2020-01-06T14:59:35Z

lib/jsonld.js

@@ -934,6 +995,27 @@ jsonld.documentLoaders = {};
 jsonld.documentLoaders.node = require('./documentLoaders/node');
 jsonld.documentLoaders.xhr = require('./documentLoaders/xhr');

+// Optional DOM parser
+try {
+  jsonld.domParser = require('xmldom').DOMParser || class NoDOMParser {


@davidlehn -- can you comment here? I don't think we can easily support this pattern with webpack. Can you suggest an alternative path forward? Instead of a require here, the user may need to have installed another package themselves that registered a DOM parser with jsonld in a similar way we do with RDF parsers. If so -- we should copy that pattern since it's already used.

Update: PR #341 removes xmldom as a dependency.

This PR here (#347) should be updated to take that into account (specifically line 1022 here).

dlongley · 2020-01-06T15:01:18Z

Thanks @gkellogg! This is great. A few minor changes needed plus one architectural thing we need some feedback from @davidlehn on (regarding how to pull in a dom parser).

lib/util.js

gkellogg · 2020-03-03T00:36:52Z

@davidlehn are we waiting for something else to merge this?

lib/documentLoaders/xhr.js

lib/jsonld.js

lib/documentLoaders/node.js

davidlehn · 2020-03-03T02:11:54Z

@gkellogg I was waiting on me to look into how to improve the way the xmldom lib is used. Otherwise I think patch is good. As Dave's comment mentioned, might want some sort of registered parser pattern. Need to check how the current code works with bundlers and such.

* Adds options parameter to documentLoader * Uses xmldom, if loaded. * Adds util.ParseContentTypeHeader * Adds documentLoader implementations for xhr and node (still requires tests).

Skip HTML tests if there is no DOMParser, or loading the module raises an exception. Allows Karma tests to pass.

Co-Authored-By: Dave Longley <dlongley@digitalbazaar.com>

… method.

Co-Authored-By: David I. Lehn <dil@lehn.org>

- Updated tests switch from "invalid script element" to "loading document failed".

gkellogg requested review from davidlehn and dlongley December 28, 2019 01:35

gkellogg commented Dec 28, 2019

View reviewed changes

gkellogg force-pushed the load-html branch from 87e34e7 to 7841e01 Compare December 31, 2019 19:23

davidlehn added this to the JSON-LD 1.1 milestone Dec 31, 2019

gkellogg marked this pull request as ready for review January 5, 2020 21:46

dlongley requested changes Jan 6, 2020

View reviewed changes

gkellogg requested a review from dlongley January 6, 2020 20:23

gkellogg force-pushed the load-html branch from 99efe03 to a9db562 Compare January 7, 2020 21:52

gkellogg force-pushed the load-html branch 2 times, most recently from c1c0221 to d1c1290 Compare January 26, 2020 20:59

gkellogg force-pushed the load-html branch 2 times, most recently from b0f158d to a45ef6d Compare January 29, 2020 01:01

davidlehn reviewed Jan 30, 2020

View reviewed changes

lib/util.js Outdated Show resolved Hide resolved

davidlehn mentioned this pull request Feb 28, 2020

Implement JSON-LD 1.1 embedded in HTML. #298

Open

gkellogg force-pushed the load-html branch from 84abcd8 to a70f73a Compare March 3, 2020 00:32

davidlehn reviewed Mar 3, 2020

View reviewed changes

lib/documentLoaders/xhr.js Outdated Show resolved Hide resolved

davidlehn reviewed Mar 3, 2020

View reviewed changes

lib/jsonld.js Outdated Show resolved Hide resolved

davidlehn reviewed Mar 3, 2020

View reviewed changes

lib/documentLoaders/node.js Outdated Show resolved Hide resolved

gkellogg force-pushed the load-html branch from 05c0487 to d33ad5f Compare March 5, 2020 01:29

davidlehn force-pushed the load-html branch from d33ad5f to bcf3bee Compare March 6, 2020 05:06

gkellogg and others added 18 commits April 9, 2020 14:58

Load JSON-LD from HTML documents.

d25c882

* Adds options parameter to documentLoader * Uses xmldom, if loaded. * Adds util.ParseContentTypeHeader * Adds documentLoader implementations for xhr and node (still requires tests).

Set default for expandAllScripts to true for flatten and toRdf.

7730b10

Fix lint errors.

59da13f

Don't extract all scripts if there is a fragment identifier.

bdc9343

Update changelog

7b29cb3

Add contentType check to XHR documentLoader link header test.

a41f081

Test for DOMParser slightly more complicated.

03c4834

Skip HTML tests if there is no DOMParser, or loading the module raises an exception. Allows Karma tests to pass.

Fix CHANGELOG.

5fa1b2a

Apply suggestions from @dlongley code review

5315d67

Co-Authored-By: Dave Longley <dlongley@digitalbazaar.com>

More changes suggested by @dlongley.

31f192d

Fix changelog.

9d5ec19

Use content-type package, instead of purpose-defined parseContentType…

2664ac7

… method.

Update lib/documentLoaders/xhr.js

868dae6

Co-Authored-By: David I. Lehn <dil@lehn.org>

Update lib/documentLoaders/node.js

e38ba63

Co-Authored-By: David I. Lehn <dil@lehn.org>

Update lib/jsonld.js

9a9bbdb

Co-Authored-By: David I. Lehn <dil@lehn.org>

Fix lint issues.

7cc3d1c

Update error code.

66a2092

- Updated tests switch from "invalid script element" to "loading document failed".

Error when loading HTML and no script element exists.

11b68b9

gkellogg force-pushed the load-html branch from 073b3c1 to 11b68b9 Compare April 9, 2020 22:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Load html #347

Load html #347

gkellogg commented Dec 28, 2019

gkellogg commented Dec 28, 2019

gkellogg Dec 28, 2019

gkellogg Dec 28, 2019

BigBlueHat Jan 24, 2020

rubensworks Mar 28, 2020

gkellogg Dec 28, 2019

gkellogg Dec 28, 2019

gkellogg Dec 28, 2019

gkellogg commented Dec 28, 2019

dlongley Jan 6, 2020

dmitrizagidulin Feb 10, 2021

dlongley commented Jan 6, 2020

gkellogg commented Mar 3, 2020

davidlehn commented Mar 3, 2020

Load html #347

Are you sure you want to change the base?

Load html #347

Conversation

gkellogg commented Dec 28, 2019

gkellogg commented Dec 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkellogg commented Dec 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlongley commented Jan 6, 2020

gkellogg commented Mar 3, 2020

davidlehn commented Mar 3, 2020