make_names.pl should always generate all names in Names.* files

• Component: DOM
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

make_names.pl should always generate all names in Names.* files

[Eric Seidel (no email)]

Created attachment 61148 [details]
Patch

[Eric Seidel (no email)]

The next step after this will be to remove a bunch of (now unnecessary) #if guards.  Then I'll need to go through and fix various build systems to always generate SVGNames, etc.

It would actually be really helpful if someone from the Qt side of things could fix SVGNames to always be generated even when SVG is off.  The files are always guarded with #if ENABLE(SVG) anyway, so it should not change what's actually built (and will make my job in the next step much easier).

[Adam Barth]

Comment on attachment 61148 [details]
Patch

I wanted to review this change, but I don't understand Perl well enough.

[Csaba Osztrogonác]

Unfortunately I can't help, because I will go on a holiday next week and will be offline till 19th July. I cc-ed my collegue, Andras. I think he can help you with Qt build system.

[Darin Adler]

Comment on attachment 61148 [details]
Patch

Not generating names for features that are disabled makes certain classes of programming mistakes impossible -- you can’t accidentally compile in some of the code that uses those names -- and also makes the code slightly smaller.

What’s the benefit of generating all the names all the time?

[Eric Seidel (no email)]

(In reply to comment #5)
> (From update of attachment 61148 [details])
> Not generating names for features that are disabled makes certain classes of programming mistakes impossible -- you can’t accidentally compile in some of the code that uses those names -- and also makes the code slightly smaller.

This is true.

> What’s the benefit of generating all the names all the time?

Always generating all names, makes some programming easier too. :)  Not quite as many ifs need to be broken out by #if ENABLE(FOO) blocks.

But the real reason is, that the HTML5 parser has special treatment for some SVG and MathML elements and attributes.  We need QualifiedNames for those attributes.  It seems silly to define our own in the parser for the cases where SVG or MathML is not enabled.

Another option would be to simply not support those elements or atributes in the parser when SVG/MathML were disabled.  But I think that's worse, as then we parse documents differently based on whether we render SVG or MathML.

Thus I suggest that we always generate all names and use them throughout our code without #if ENABLE(FOO).  The actual ElementFactories should also be generated, but will always return 0 if called in when the various technologies are disabled.

[Eric Seidel (no email)]

Our current behavior in the new HTMLTreeBuilder is to place all the sVG and MathML code in #if blocks. Thus the checke in results:

http://trac.webkit.org/browser/trunk/LayoutTests/html5lib/runner-expected-html5.txt

Pass most of the SVG-related tests, and fail all of the MathML ones.  We'd like to pass parser tests all the time, regardless of what rendering abilities may be enabled in the engine.

[Darin Adler]

(In reply to comment #6)
> Another option would be to simply not support those elements or atributes in the parser when SVG/MathML were disabled.  But I think that's worse, as then we parse documents differently based on whether we render SVG or MathML.

I don’t see this as a negative the way you do. It seems strange to have parser knowledge of these languages included when the language is otherwise omitted entirely from WebKit.

[Anne van Kesteren]

Parsing SVG/MathML correctly per HTML5 is not an optional feature however. At least not at this point.

[Darin Adler]

(In reply to comment #9)
> Parsing SVG/MathML correctly per HTML5 is not an optional feature however. At least not at this point.

So the concept is that parsing it is must be correct, even if not creating the correct element types? I guess that makes sense. It's hard for me to care a lot about this, since I think WebKit implementations should not compile out the SVG and MathML.

[Adam Barth]

One way to think about it is we parse mathml and svg correctly on XML even if we don't implement those features.  :)

[Eric Seidel (no email)]

It is also possible to wrap the ElementFactory* and Names* files in different ENABLE(FOO) blocks.  Names could be ENABLE(FOO_NAMES) and we could always have FOO_NAMES defined for SVG, XML, XLink, XMLNS and MathML even if ports disable SVG or MathML.  That would leave WMLNames off if that's a concern.  Nearly all of SVGNames and MathMLNames are all required and all of  XMLNames, XLinkNames, XMLNSNames are required for proper parsing of HTML5.

Then again, ports which never wish to have a feature optional (like Mac for WML) don't need to include the *Names or *ElementFactory.cpp files at all.

[Eric Seidel (no email)]

See bug 42050 for the next phase of this.

[Eric Seidel (no email)]

Darin r+'d the bigger-deal bug 42050 change, but this one still needs someone's rubber stamp.

A reminder of what it does:

- Enables all SVGNames, all the time.  Regardless of what SVG sub-features may be turned on.  For example, SVGNames::foreignObjectTag will always be defined if SVG is on, regardless of whether SVG_FOREIGN_OBJECT is defined.  (Bug 42050, makes it so that SVGNames is then always defined regardless of whether SVG is enabled or not.)

- Changes make_names (mostly search-replace) to support the above.  Does so by reading the .in files twice.  Once through the pre-processor, and once without.  To support such, I added new parsedTags and parsedAttrs variants on "attrs" and "tags" dictionaries.  The restults of those parses are then stuffed into "allTags" or "enabledTags" dictionaries, which are then used by *Names and *ElementFactory generation, respectively.

[Adam Barth]

Comment on attachment 61148 [details]
Patch

It's slightly tricky to see what's going on in this patch because of the renaimings, but this looks reasonable now that the other bug has been resolved.

[Eric Seidel (no email)]

Committed r63109: <http://trac.webkit.org/changeset/63109>

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.