Closed
Bug 1100206
Opened 10 years ago
Closed 9 years ago
Teach the parser about the integrity attribute
Categories
(Core :: DOM: HTML Parser, defect)
Core
DOM: HTML Parser
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: francois, Assigned: francois)
References
Details
Attachments
(2 files, 1 obsolete file)
4.00 KB,
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
49.69 KB,
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
This attribute is defined by the sub-resource integrity specification: https://w3c.github.io/webappsec/specs/subresourceintegrity/
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → francois
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•10 years ago
|
||
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 2•10 years ago
|
||
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+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 3•10 years ago
|
||
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!
Flags: needinfo?(francois)
Keywords: checkin-needed
Assignee | ||
Comment 4•10 years ago
|
||
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)
Comment 5•10 years ago
|
||
(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)
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8f6cfc3bbdf2
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Assignee | ||
Comment 7•10 years ago
|
||
It looks like an empty patch was committed to mozilla-central by mistake: $ hg show 216205:8f6cfc3bbdf2 changeset: 216205:8f6cfc3bbdf2 user: Francois Marier <francois@mozilla.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 → ---
Comment 8•9 years ago
|
||
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.
Attachment #8523647 -
Attachment is obsolete: true
Flags: needinfo?(hsivonen)
Attachment #8538441 -
Flags: review+
Comment 9•9 years ago
|
||
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+
Assignee | ||
Comment 10•9 years ago
|
||
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.
Keywords: checkin-needed
Comment 11•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b7cbab2c5e3
Keywords: checkin-needed
Comment 12•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7b7cbab2c5e3
Status: REOPENED → RESOLVED
Closed: 10 years ago → 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•