Bug 1482782 changed the way static atoms are defined, and the HTML parser atoms all now live in xpcom/ds/HTMLAtoms.py. The atom generation script needs to be updated for this.

Taking as I have a prelim patch for this as I was struggling to generate an attr change.

I tested this and minus Bug 1466449 it appears to be generating the right output. The non parser patch is the change it generated (mostly whitespace) I had to edit the new static .py file to be on one line so to remove a bigger change in the parser.

Comment on attachment 9018012 [details] [diff] [review] bug-1483458-parser.patch Review of attachment 9018012 [details] [diff] [review]: ----------------------------------------------------------------- r=me with that, though I don't have the original sources at hand so I will leave most of the review to Henri. ::: translator-src/nu/validator/htmlparser/cpptranslate/CppTypes.java @@ +55,4 @@ > > public class CppTypes { > > + private static final Pattern ATOM_DEF = Pattern.compile("^\\s*Atom\\(\"([^,]+)\",\\s*\"([^\"]*)\"\\).*$"); What about the atoms defined with PseudoElementAtom or NonInheritingAnonBoxAtom or InheritingAnonBoxAtom? I guess it's unlikely that we need to worry about them, but it might be worth pointing that out in a comment if so. @@ +148,4 @@ > while ((line = atomReader.readLine()) != null) { > manualAtoms.append(line); > manualAtoms.append('\n'); > + if (line.trim().startsWith("] + HTML_PARSER_ATOMS")) { It won't be obvious if someone's editing StaticAtoms.py that the "] + HTML_PARSER_ATOMS" line is important. Can you instead add some comments surrounding the HTML_PARSER_ATOMS assignment like "# START ATOMS" and "# END ATOMS" that we can check for? ::: translator-src/nu/validator/htmlparser/cpptranslate/GkAtomParser.java @@ +47,4 @@ > > public class GkAtomParser { > > + private static final Pattern ATOM = Pattern.compile("^Atom\\(\"([^,]+)\",\\s*\"([^\"]*)\"\\).*$"); Same here.

Comment on attachment 9018011 [details] [diff] [review] bug-1483458.patch Review of attachment 9018011 [details] [diff] [review]: ----------------------------------------------------------------- ::: xpcom/ds/StaticAtoms.py @@ +75,4 @@ > Atom("allowpopupstoescapesandbox", "allow-popups-to-escape-sandbox"), > Atom("allowpopups", "allow-popups"), > Atom("allowpresentation", "allow-presentation"), > + Atom("allowstorageaccessbyuseractivatetion", "allow-storage-access-by-user-activation"), Can you add a comment at the top of the file saying that all of these definitions must be on a single line because the HTML parser generation script expects it? FWIW there's a typo in the atom identifier ("activatetion"), can you file a followup for that?

Comment on attachment 9018012 [details] [diff] [review] bug-1483458-parser.patch Review of attachment 9018012 [details] [diff] [review]: ----------------------------------------------------------------- I trust heycam's comments. Thanks for fixing this.

Copying r+ over from previous patch as I fixed simple nits.

Copying r+ over from previous patch as I fixed simple nits.

Requesting checkin-needed for both patches as this should be OK to land in a soft code freeze due to there being no functional changes and is blocking anyone using this tool (probably just me): bug-1483458-parser.patch - is for https://hg.mozilla.org/projects/htmlparser/ bug-1483458.patch - is for mozilla-central/inbound etc Thank you!

Pushed by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/mozilla-inbound/rev/0ff0b54b9ec7 Change HTML parser to look at .py Atom files. r=hsivonen,heycam

Cameron, can you push this to htmlparser, please?

(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #13) > Cameron, can you push this to htmlparser, please? I've pushed it: https://hg.mozilla.org/projects/htmlparser/rev/9b24191bc606