Bug 41841

Summary: Unknown HTML elements should use the HTMLUnknownElement interface
Product: WebKit Reporter: Peter Beverloo <peter>
Component: DOMAssignee: Tom Zakrajsek <tomz>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, cmarcelo, dglazkov, gns, gustavo.noronha, tomz, webkit.review.bot, webmaster, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 32934    
Attachments:
Description Flags
Patch to implement HTMLUnknownElement
webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-03
none
Fix windows and linux builds
ap: review-
Fix test and changelog
gns: commit-queue-
Archive of layout-test-results from ec2-cr-linux-03
none
Rebase to get good build-bot run
webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-03
none
Try again with GTK
gns: commit-queue-
Fix cut+paste error in GTK build file
webkit.review.bot: commit-queue-
Fix chromium build
sam: review-, webkit.review.bot: commit-queue-
Add licenses to new files
webkit.review.bot: commit-queue-
Fix test expected results to add HTMLUnknownElement
none
Patch for landing
webkit.review.bot: commit-queue-
Fix corruption in the patchfile none

Description Peter Beverloo 2010-07-08 01:53:56 PDT
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
Comment 1 Tom Zakrajsek 2011-07-12 23:55:30 PDT
Created attachment 100636 [details]
Patch to implement HTMLUnknownElement
Comment 2 WebKit Review Bot 2011-07-13 00:20:10 PDT
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
Comment 3 WebKit Review Bot 2011-07-13 00:20:14 PDT
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
Comment 4 Collabora GTK+ EWS bot 2011-07-13 11:46:48 PDT
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
Comment 5 Tom Zakrajsek 2011-07-19 15:18:05 PDT
Created attachment 101398 [details]
Fix windows and linux builds
Comment 6 Adam Barth 2011-07-19 15:43:01 PDT
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.
Comment 7 Alexey Proskuryakov 2011-07-19 15:45:27 PDT
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.
Comment 8 Adam Barth 2011-07-19 15:47:48 PDT
> 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?
Comment 9 Alexey Proskuryakov 2011-07-19 15:53:59 PDT
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.
Comment 10 Tom Zakrajsek 2011-07-19 16:22:31 PDT
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.
Comment 11 Adam Barth 2011-07-19 16:31:18 PDT
I'd just pick some common ones, like <div> and <table>.  We don't need to list them all.
Comment 12 Tom Zakrajsek 2011-07-20 09:29:10 PDT
Created attachment 101477 [details]
Fix test and changelog

Applied feedback and re-posted.
Comment 13 Gustavo Noronha (kov) 2011-07-20 09:54:04 PDT
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
Comment 14 WebKit Review Bot 2011-07-20 09:56:18 PDT
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
Comment 15 WebKit Review Bot 2011-07-20 09:56:52 PDT
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
Comment 16 Tom Zakrajsek 2011-07-20 12:44:38 PDT
Created attachment 101496 [details]
Rebase to get good build-bot run
Comment 17 WebKit Review Bot 2011-07-20 13:13:58 PDT
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
Comment 18 WebKit Review Bot 2011-07-20 13:14:30 PDT
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
Comment 19 Collabora GTK+ EWS bot 2011-07-20 14:18:09 PDT
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
Comment 20 Tom Zakrajsek 2011-07-20 14:40:56 PDT
Created attachment 101512 [details]
Try again with GTK
Comment 21 Gustavo Noronha (kov) 2011-07-20 16:11:23 PDT
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
Comment 22 Tom Zakrajsek 2011-07-21 10:47:29 PDT
Created attachment 101604 [details]
Fix cut+paste error in GTK build file
Comment 23 WebKit Review Bot 2011-07-21 12:33:29 PDT
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
Comment 24 Tom Zakrajsek 2011-08-05 17:08:55 PDT
Created attachment 103132 [details]
Fix chromium build
Comment 25 WebKit Review Bot 2011-08-05 17:53:37 PDT
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
Comment 26 Sam Weinig 2011-08-05 22:29:58 PDT
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.
Comment 27 Tom Zakrajsek 2011-08-09 17:15:10 PDT
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.
Comment 28 WebKit Review Bot 2011-08-09 18:21:32 PDT
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
Comment 29 Tom Zakrajsek 2011-08-10 21:14:45 PDT
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
Comment 30 Adam Barth 2011-08-11 10:13:00 PDT
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.
Comment 31 Tom Zakrajsek 2011-08-11 12:06:23 PDT
Created attachment 103652 [details]
Patch for landing
Comment 32 WebKit Review Bot 2011-08-11 12:07:20 PDT
Comment on attachment 103652 [details]
Patch for landing

Rejecting attachment 103652 [details] from commit-queue.

tomz@codeaurora.org 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.
Comment 33 Adam Barth 2011-08-11 12:40:02 PDT
Comment on attachment 103652 [details]
Patch for landing

I suspect this won't work because none of the bots could apply the patch.
Comment 34 WebKit Review Bot 2011-08-11 12:44:03 PDT
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
Comment 35 Tom Zakrajsek 2011-08-11 15:03:37 PDT
Created attachment 103681 [details]
Fix corruption in the patchfile
Comment 36 WebKit Review Bot 2011-08-11 16:35:44 PDT
Comment on attachment 103681 [details]
Fix corruption in the patchfile

Clearing flags on attachment: 103681

Committed r92890: <http://trac.webkit.org/changeset/92890>
Comment 37 WebKit Review Bot 2011-08-11 16:36:10 PDT
All reviewed patches have been landed.  Closing bug.