Fixed bugs in embed and remote frame context handling. - #31
Closed
RDProjekt wants to merge 2 commits into
Closed
Conversation
added 2 commits
July 24, 2014 16:11
Added unit-tests to cover this scenario.
Member
|
Thanks @RDProjekt! We need to take a closer look at this when we can, but it does look like we want to pull it in. We do need to get generic unit tests (that aren't just the official JSON-LD test suite) working and running in travis so we can throw more in there as needed and this could be a good start for that. We'd like, at least eventually, all of the tests (official JSON-LD test suite and our own custom ones) to run at the same time. |
Member
|
It would be good to try and get these tests into the main test suite. We want to be able test all implementations for these issues. |
12 tasks
Collaborator
|
This fix and tests are now included in #228 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi!
We noticed two bugs in your code. One is in the framing algorithm when the remote frame contains a context url (the "if" was never True and if/else branches were reversed), and the other is in the _remove_embed function (lack of "enumerate" while iterating embed['parent']).
Here's a pull request with patches. We also added some unit-tests for the first bug. Because json-ld-org tests don't cover it, we had to come up with our own.
Best regards,
RD Projekt