When I try to regenerate the parser per the directions in parser/html/java/README.txt I get: Missing SVG element: set_ and the resulting output has the wrong constructor function for the element. This was introduced by bug 1445117, apparently... I suspect that what happened is that the parser generator looked through nsGkAtoms and found the set_ thing that was present there, which is why the generated code ended up using it. But changing the element _name_ in our element lists so it no longer matches known element names is not the way to go here. The right answer was to just regenerate the parser.

Since this is hard-blocking me right now....

And the nsGkAtoms.h move broke the regeneration makefile, though that's at least a simple fix.

Apologies for the breakage. It's hard to avoid when there are obscure dependencies that aren't tested on automation. Note also that nsGkAtoms.cpp moved from dom/base/ to xpcom/ds/. TBH I'm pretty fed up with the arrangement of the HTML5 parser. The presence of generated code in mozilla-central that is generated from an external repo is very hostile to development. It has caused me multiple problems recently.

njn, could you review the non-parser bits? Henri, can you review the parser bits?

Comment on attachment 8959279 [details] [diff] [review] Switch to "set" as the canonical nsGkAtoms name of the string "set", so it matches the actual tag name "set" in SVG Review of attachment 8959279 [details] [diff] [review]: ----------------------------------------------------------------- r=me for the non-parser parts. ::: xpcom/ds/nsGkAtomList.h @@ -1990,5 @@ > GK_ATOM(separator_, "separator") > GK_ATOM(separators_, "separators") > GK_ATOM(sep_, "sep") > GK_ATOM(setdiff_, "setdiff") > -GK_ATOM(set_, "set") For consistency with other cases like this, instead of deleting this entry I would comment it out, and add a '"set" is present above" comment.

> instead of deleting this entry I would comment it out, and add a '"set" is present above" comment. Why? Who would ever go looking for "nsGkAtoms::set_"? I mean, I'm happy to do this if there's a reason, but is there one?

Maybe the header reordering is the parser generation code, not clang-format, by the way.

Apparently the header-reordering, whoever did it, does not compile. Updated patch coming up with that removed....

Comment on attachment 8959279 [details] [diff] [review] Switch to "set" as the canonical nsGkAtoms name of the string "set", so it matches the actual tag name "set" in SVG r+ on the assumption that this 1) changes the set atom 2) changes the atom file path 3) the rest comes from clang-format

> 1) changes the set atom Yes. > 2) changes the atom file path Yes. > 3) the rest comes from clang-format Some of it comes from the parser generating the cpp files... for example the header reorderings sure do.

> Why? Who would ever go looking for "nsGkAtoms::set_"? I mean, I'm happy to > do this if there's a reason, but is there one? Is this list that "set" is part of -- "saturation", "saturate", "set", "seed", etc. -- from some predefined list of things? If so, it makes sense to leave it in. But if that list has just accumulated over time, then I agree that removing it is reasonable.

Looks like "set" was added for the SVG <set> tag. It's just there in alphabetical order in the original gkatoms list. The "set_" bit is for the MathML element. It used to be in a separate atom list and then got merged into gkatoms and then renamed. I guess given that, it makes sense to just leave the commented-out thing in the mathml element, I guess.

Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6af1f5ac596b Switch to "set" as the canonical nsGkAtoms name of the string "set", so it matches the actual tag name "set" in SVG. r=hsivonen,njn