Bug 41841 – Unknown HTML elements should use the HTMLUnknownElement interface

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Peter Beverloo]

Per the HTML5 specification[1], any element which are not defined by any of the specifications should use the HTMLUnknownElement interface rather than the HTMLElement interface, as currently is the case on WebKit. Opera and Firefox already implement this behaviour, and according to MSDN[2] Internet Explorer 8 does this as well.

[1] http://www.w3.org/TR/html5/elements.html#elements-in-the-dom
[2] http://msdn.microsoft.com/en-us/library/cc849021(VS.85).aspx

[Tom Zakrajsek]

Created attachment 100636 [details]
Patch to implement HTMLUnknownElement

[WebKit Review Bot]

Comment on attachment 100636 [details]
Patch to implement HTMLUnknownElement

Attachment 100636 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9017455

New failing tests:
http/tests/cookies/simple-cookies-expired.html
http/tests/canvas/webgl/origin-clean-conformance.html
dom/html/level1/core/hc_attrappendchild3.html
animations/animation-events-create.html
animations/computed-style.html
animations/keyframes-rule.html
css3/unicode-bidi-insolate-parse.html
http/tests/cookies/simple-cookies-max-age.html
dom/html/level1/core/hc_attrappendchild4.html
animations/animation-css-rule-types.html
http/tests/cookies/multiple-cookies.html
css1/units/rounding.html
http/tests/history/back-during-onload-triggered-by-back.html
http/tests/filesystem/resolve-uri.html
dom/html/level1/core/hc_attrappendchild2.html
dom/html/level1/core/hc_attrappendchild5.html
dom/html/level2/core/setAttributeNS10.html
http/tests/history/popstate-fires-with-pending-requests.html
css3/zoom-coords.xhtml
dom/html/level1/core/hc_attrappendchild1.html

[WebKit Review Bot]

Created attachment 100638 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Collabora GTK+ EWS bot]

Comment on attachment 100636 [details]
Patch to implement HTMLUnknownElement

Attachment 100636 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9050225

[Tom Zakrajsek]

Created attachment 101398 [details]
Fix windows and linux builds

[Adam Barth]

Comment on attachment 101398 [details]
Fix windows and linux builds

View in context: https://bugs.webkit.org/attachment.cgi?id=101398&action=review

I'm not an expert on make_names, but your change looks reasonably likely to be correct.

> LayoutTests/fast/html/script-tests/unknown-tag.js:25
> +// These tags are required by the HTML spec
> +var validTags = [
> +    "a", "abbr", "acronym", "address", "applet", "area",
> +    "article", "aside", "b", "base", "basefont", "bdo",
> +    "bgsound", "big", "blockquote", "body", "br", "button",
> +    "canvas", "caption", "center", "cite", "code", "col",
> +    "colgroup", "command", "dd", "del",
> +    "dfn", "dir", "div", "dl", "dt", "em", "embed", "fieldset",
> +    "figcaption", "figure", "font", "footer", "form", "frame",
> +    "frameset", "h1", "h2", "h3", "h4", "h5", "h6", "head",
> +    "header", "hgroup", "hr", "html", "i", "iframe", "image",
> +    "img", "input", "ins", "isindex", "kbd", "keygen", "label",
> +    "layer", "legend", "li", "link", "listing", "map", "mark",
> +    "marquee", "menu", "meta", "nav", "nobr",
> +    "noembed", "noframes", "nolayer", "noscript", "object",
> +    "ol", "optgroup", "option", "output", "p", "param",
> +    "plaintext", "pre", "q", "rp", "rt", "ruby",
> +    "s", "samp", "script", "section", "select", "small",
> +    "span", "strike", "strong", "style", "sub",
> +    "sup", "table", "tbody", "td", "textarea",
> +    "tfoot", "th", "thead", "title", "tr", "tt", "u",
> +    "ul", "var", "wbr", "xmp"
> +];

This seems like a pain to maintain this list.

> Source/WebCore/ChangeLog:36
> -        
> +
>          It was possible to recurse via updateGeometry/swapFromOrToTiledLayer/
>          updateContentsScale because updateGeometry() and updateContentsScale()
>          used different sizes; updateGeometry() used the scaled size, while
>          updateContentsScale() used the unscaled size.
> -        
> +

These diffs shouldn't be here.

[Alexey Proskuryakov]

Comment on attachment 101398 [details]
Fix windows and linux builds

Oh, just one thing - please don't make a script-test. This is a anti-pattern.

The .js part should just be included inside .html.

[Adam Barth]

> Oh, just one thing - please don't make a script-test. This is a anti-pattern.

I didn't realize that using we were moving away from using script-tests.  Should we write a script to bulk-convert the existing (non-JS, I presume) tests?

[Alexey Proskuryakov]

I've been hoping to do that (and even more importantly, to prevent make-script-test-wrappers from creating new ones outside fast/js) for a long one, but I keep being sidetracked. Seems unlikely that I'll get to that soon.

[Tom Zakrajsek]

Comment on attachment 101398 [details]
Fix windows and linux builds

View in context: https://bugs.webkit.org/attachment.cgi?id=101398&action=review

>> LayoutTests/fast/html/script-tests/unknown-tag.js:25
>> +];
> 
> This seems like a pain to maintain this list.

I agree but couldn't think of a better way to do this.  The test needs at least /some/ known element names to verify that we're not supporting HTMLUnknownElement for known elements.  I listed all of our unconditional ones because there are some branch differences through the make_names.pl for different classes of tags.  Does it make sense to skip testing for this inappropriate support for HTMLUnknownElement?

>> Source/WebCore/ChangeLog:36

> 
> These diffs shouldn't be here.

Yeah, not sure why I did that.  I'll remove it.

[Adam Barth]

I'd just pick some common ones, like <div> and <table>.  We don't need to list them all.

[Tom Zakrajsek]

Created attachment 101477 [details]
Fix test and changelog

Applied feedback and re-posted.

[Gustavo Noronha (kov)]

Comment on attachment 101477 [details]
Fix test and changelog

Attachment 101477 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9200317

[WebKit Review Bot]

Comment on attachment 101477 [details]
Fix test and changelog

Attachment 101477 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9193316

New failing tests:
http/tests/cookies/simple-cookies-expired.html
http/tests/canvas/webgl/origin-clean-conformance.html
http/tests/inspector/console-xhr-logging.html
animations/computed-style.html
animations/keyframes-rule.html
http/tests/inspector/change-iframe-src.html
css3/unicode-bidi-insolate-parse.html
http/tests/cookies/simple-cookies-max-age.html
http/tests/inspector/network-preflight-options.html
http/tests/inspector/inspect-iframe-from-different-domain.html
animations/animation-css-rule-types.html
http/tests/cookies/multiple-cookies.html
css1/units/rounding.html
http/tests/history/back-during-onload-triggered-by-back.html
http/tests/filesystem/resolve-uri.html
http/tests/inspector/resource-har-conversion.html
http/tests/inspector/console-resource-errors.html
http/tests/history/popstate-fires-with-pending-requests.html
css3/zoom-coords.xhtml
animations/animation-events-create.html

[WebKit Review Bot]

Created attachment 101480 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Tom Zakrajsek]

Created attachment 101496 [details]
Rebase to get good build-bot run

[WebKit Review Bot]

Comment on attachment 101496 [details]
Rebase to get good build-bot run

Attachment 101496 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9199414

New failing tests:
http/tests/cookies/simple-cookies-expired.html
http/tests/canvas/webgl/origin-clean-conformance.html
http/tests/inspector/console-xhr-logging.html
animations/computed-style.html
animations/keyframes-rule.html
http/tests/inspector/change-iframe-src.html
css3/unicode-bidi-insolate-parse.html
http/tests/cookies/simple-cookies-max-age.html
http/tests/inspector/network-preflight-options.html
http/tests/inspector/inspect-iframe-from-different-domain.html
animations/animation-css-rule-types.html
http/tests/cookies/multiple-cookies.html
css1/units/rounding.html
http/tests/history/back-during-onload-triggered-by-back.html
http/tests/filesystem/resolve-uri.html
http/tests/inspector/resource-har-conversion.html
http/tests/inspector/console-resource-errors.html
http/tests/history/popstate-fires-with-pending-requests.html
css3/zoom-coords.xhtml
animations/animation-events-create.html

[WebKit Review Bot]

Created attachment 101499 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Collabora GTK+ EWS bot]

Comment on attachment 101496 [details]
Rebase to get good build-bot run

Attachment 101496 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9201435

[Tom Zakrajsek]

Created attachment 101512 [details]
Try again with GTK

[Gustavo Noronha (kov)]

Comment on attachment 101512 [details]
Try again with GTK

Attachment 101512 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9201483

[Tom Zakrajsek]

Created attachment 101604 [details]
Fix cut+paste error in GTK build file

[WebKit Review Bot]

Comment on attachment 101604 [details]
Fix cut+paste error in GTK build file

Attachment 101604 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9201844

New failing tests:
http/tests/cookies/simple-cookies-expired.html
http/tests/canvas/webgl/origin-clean-conformance.html
http/tests/inspector/console-websocket-error.html
http/tests/inspector/console-xhr-logging.html
animations/computed-style.html
animations/keyframes-rule.html
http/tests/inspector/change-iframe-src.html
css3/unicode-bidi-insolate-parse.html
http/tests/cookies/simple-cookies-max-age.html
http/tests/inspector/network-preflight-options.html
http/tests/inspector/inspect-iframe-from-different-domain.html
animations/animation-css-rule-types.html
http/tests/cookies/multiple-cookies.html
css1/units/rounding.html
http/tests/history/back-during-onload-triggered-by-back.html
http/tests/filesystem/resolve-uri.html
http/tests/inspector/console-resource-errors.html
http/tests/history/popstate-fires-with-pending-requests.html
css3/zoom-coords.xhtml
animations/animation-events-create.html

[Tom Zakrajsek]

Created attachment 103132 [details]
Fix chromium build

[WebKit Review Bot]

Comment on attachment 103132 [details]
Fix chromium build

Attachment 103132 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9321265

New failing tests:
fast/dom/prototype-inheritance.html

[Sam Weinig]

Comment on attachment 103132 [details]
Fix chromium build

View in context: https://bugs.webkit.org/attachment.cgi?id=103132&action=review

r- for missing licenses.

> Source/WebCore/html/HTMLUnknownElement.h:7
> +
> +#ifndef HTMLUnknownElement_h
> +#define HTMLUnknownElement_h
> +
> +#include "HTMLElement.h"
> +
> +namespace WebCore {

This file is missing a license.

> Source/WebCore/html/HTMLUnknownElement.idl:6
> +module html {
> +
> +    interface HTMLUnknownElement : HTMLElement {
> +    };
> +
> +}

This file is also missing a license.

[Tom Zakrajsek]

Created attachment 103422 [details]
Add licenses to new files

test bots appear flaky for chrome.  I see fluctuating errors on mac-chrome with or without these changes and they seem unrelated to anything I changed.  I'm looking at the prototype-inheritance failure.

[WebKit Review Bot]

Comment on attachment 103422 [details]
Add licenses to new files

Attachment 103422 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9339435

New failing tests:
fast/dom/prototype-inheritance.html

[Tom Zakrajsek]

Created attachment 103575 [details]
Fix test expected results to add HTMLUnknownElement

not sure yet why this only caused a problem for the chromium-linux bot

[Adam Barth]

Comment on attachment 103575 [details]
Fix test expected results to add HTMLUnknownElement

View in context: https://bugs.webkit.org/attachment.cgi?id=103575&action=review

> Source/WebCore/WebCore.vcproj/WebCore.vcproj:6776
> +			<File				RelativePath="$(ConfigurationBuildDir)\obj\$(ProjectName)\DerivedSources\JSHTMLElementWrapperFactory.h"

This looks kind of wonky.

[Tom Zakrajsek]

Created attachment 103652 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 103652 [details]
Patch for landing

Rejecting attachment 103652 [details] from commit-queue.

[e-mail] does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.

[Adam Barth]

Comment on attachment 103652 [details]
Patch for landing

I suspect this won't work because none of the bots could apply the patch.

[WebKit Review Bot]

Comment on attachment 103652 [details]
Patch for landing

Rejecting attachment 103652 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 2

Last 500 characters of output:
Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
patching file Source/WebCore/dom/make_names.pl
patching file Source/WebCore/html/HTMLTagNames.in
patching file Source/WebCore/html/HTMLUnknownElement.h
patching file Source/WebCore/html/HTMLUnknownElement.idl
patching file Source/WebCore/mathml/mathtags.in
patching file Source/WebCore/page/DOMWindow.idl
patching file Source/WebCore/svg/svgtags.in

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 2

Full output: http://queues.webkit.org/results/9346306

[Tom Zakrajsek]

Created attachment 103681 [details]
Fix corruption in the patchfile

[WebKit Review Bot]

Comment on attachment 103681 [details]
Fix corruption in the patchfile

Clearing flags on attachment: 103681

Committed r92890: <http://trac.webkit.org/changeset/92890>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.