Closed Bug 1100206 Opened 7 years ago Closed 7 years ago
Teach the parser about the integrity attribute
This attribute is defined by the sub-resource integrity specification: https://w3c.github.io/webappsec/specs/subresourceintegrity/
Assignee: nobody → francois
Status: NEW → ASSIGNED
Henri, I'm putting this patch up for review here because I've tested it in bug 992096 and it looks very much in line with similar commits I've seen in the hg history. However, there are two caveats: 1. I haven't been able to figure out how to run tests in the htmlparser repo. 2. I don't know where Doug got the magic number from. I'd love to be educated on these two points because I couldn't find any documentation as to what the magic numbers are referring to, and running "mvn test" said that "Tests are skipped".
Attachment #8523647 - Flags: review?(hsivonen)
Comment on attachment 8523647 [details] [diff] [review] Add integrity attribute (In reply to François Marier [:francois] from comment #1) > Created attachment 8523647 [details] [diff] [review] > Add integrity attribute > > Henri, I'm putting this patch up for review here because I've tested it in > bug 992096 and it looks very much in line with similar commits I've seen in > the hg history. > > However, there are two caveats: > > 1. I haven't been able to figure out how to run tests in the htmlparser repo. To run the tests, you need a checkout of https://github.com/html5lib/html5lib-tests/ and then you give the relevant test file paths as arguments to TreeTester or TokenizerTester. > 2. I don't know where Doug got the magic number from. The magic number comes from uncommenting some code in AttributeName.java and running the file as a Java program. Assuming that Doug generated the number that way, it should be OK, so r+. The number is used for efficient lookup of the objects.
Attachment #8523647 - Flags: review?(hsivonen) → review+
Hi Francois, the patch failed to apply: applying bug1100206.patch unable to find 'src/nu/validator/htmlparser/impl/AttributeName.java' for patching 3 out of 3 hunks FAILED -- saving rejects to file src/nu/validator/htmlparser/impl/AttributeName.java.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir could you take a look at this ? Thanks! Also if possible could you include a try link thanks!
Sorry, I forgot to say that this patch is not for mozilla-central, but rather for https://hg.mozilla.org/projects/htmlparser/ I'm guessing I shouldn't have used the checkin-needed keyword right?
Flags: needinfo?(francois) → needinfo?(cbook)
(In reply to François Marier [:francois] from comment #4) > Sorry, I forgot to say that this patch is not for mozilla-central, but > rather for https://hg.mozilla.org/projects/htmlparser/ > > I'm guessing I shouldn't have used the checkin-needed keyword right? good question, hsivonen could you do the checkin ?
Flags: needinfo?(cbook) → needinfo?(hsivonen)
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
It looks like an empty patch was committed to mozilla-central by mistake: $ hg show 216205:8f6cfc3bbdf2 changeset: 216205:8f6cfc3bbdf2 user: Francois Marier <email@example.com> date: Sun Nov 16 20:12:00 2014 +0100 description: Mozilla bug 1100206 - Teach the parser about the integrity attribute I guess there's no need to revert it since it's an empty commit, but this is meant to go on the htmlparser repo.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Did the previously attached patch really come from code generation? The magic number isn't sorted right. Just to be sure, I ran AttributeName generation again and landed the re-generated result: https://hg.mozilla.org/projects/htmlparser/rev/903fa49d6fa8 Attaching what actually got landed on htmlparser.
Here's the m-c patch. I put you in as the hg user, since there's really no authorship from me here. I just re-ran the generator. I didn't land it on m-c, though, in case you want to land it yourself.
Attachment #8538443 - Flags: review+
Thanks Henri! I don't actually have push access to mozilla-central, so I'll mark the "Generated code to land on m-c" as "checkin-needed.
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.