Fix serialization of keygen and track elements to match the specification: - https://html.spec.whatwg.org/#serialising-html-fragments They are not supposed to have an end tag. Firefox and Chrome agree with the specification.
Created attachment 289474 [details] Patch
Comment on attachment 289474 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=289474&action=review Does this leave us with sufficient test coverage? I can’t easily see the difference between this list and the ieForbidsInsertHTML list. > Source/WebCore/editing/MarkupAccumulator.cpp:615 > + return element.hasTagName(areaTag) > + || element.hasTagName(baseTag) > + || element.hasTagName(basefontTag) > + || element.hasTagName(bgsoundTag) > + || element.hasTagName(brTag) > + || element.hasTagName(colTag) > + || element.hasTagName(embedTag) > + || element.hasTagName(frameTag) > + || element.hasTagName(hrTag) > + || element.hasTagName(imgTag) > + || element.hasTagName(inputTag) > + || element.hasTagName(keygenTag) > + || element.hasTagName(linkTag) > + || element.hasTagName(metaTag) > + || element.hasTagName(paramTag) > + || element.hasTagName(sourceTag) > + || element.hasTagName(trackTag) > + || element.hasTagName(wbrTag); An unrolled set of checks like this might be less efficient than using an array of the tags with a loop when there are so many tags involved; it might even be worth considering a HashSet, but probably not.
Comment on attachment 289474 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=289474&action=review >> Source/WebCore/editing/MarkupAccumulator.cpp:615 >> + || element.hasTagName(wbrTag); > > An unrolled set of checks like this might be less efficient than using an array of the tags with a loop when there are so many tags involved; it might even be worth considering a HashSet, but probably not. Compared to ieForbidsInsertHTML(), this list adds bgsound, keygen, and track. It removes <image>. -> We have newly passing tests for bgsound, keygen, and track. -> For <image>, it is an SVGElement, not an HTMLElement. Therefore, it would have returned early at the beginning of this function. I'll look into using an array / loop.
Comment on attachment 289474 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=289474&action=review >>> Source/WebCore/editing/MarkupAccumulator.cpp:615 >>> + || element.hasTagName(wbrTag); >> >> An unrolled set of checks like this might be less efficient than using an array of the tags with a loop when there are so many tags involved; it might even be worth considering a HashSet, but probably not. > > Compared to ieForbidsInsertHTML(), this list adds bgsound, keygen, and track. It removes <image>. > -> We have newly passing tests for bgsound, keygen, and track. > -> For <image>, it is an SVGElement, not an HTMLElement. Therefore, it would have returned early at the beginning of this function. > > I'll look into using an array / loop. Sounds good. I know it’s not normal, but are you sure you can’t create an element with the HTML namespace but with the tag "image"? I guess there is some trick that converts "image" into imgTag during normal parsing. And I suppose this is for HTML, not XML documents, but still wondering if there is some way to create such an element.
(In reply to comment #4) > Comment on attachment 289474 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=289474&action=review > > >>> Source/WebCore/editing/MarkupAccumulator.cpp:615 > >>> + || element.hasTagName(wbrTag); > >> > >> An unrolled set of checks like this might be less efficient than using an array of the tags with a loop when there are so many tags involved; it might even be worth considering a HashSet, but probably not. > > > > Compared to ieForbidsInsertHTML(), this list adds bgsound, keygen, and track. It removes <image>. > > -> We have newly passing tests for bgsound, keygen, and track. > > -> For <image>, it is an SVGElement, not an HTMLElement. Therefore, it would have returned early at the beginning of this function. > > > > I'll look into using an array / loop. > > Sounds good. > > I know it’s not normal, but are you sure you can’t create an element with > the HTML namespace but with the tag "image"? I guess there is some trick > that converts "image" into imgTag during normal parsing. And I suppose this > is for HTML, not XML documents, but still wondering if there is some way to > create such an element. Sure, it is possible to do document.createElement("image") and get an HTMLUnknownElement that the tag image. I have likely changed the behavior in such case (although the behavior change is a good thing here). I'll add test coverage and make sure other browsers pass this test).
Created attachment 289510 [details] Patch
The commit-queue encountered the following flaky tests while processing attachment 289510 [details]: fast/images/move-image-to-new-document.html bug 162372 (authors: arv@chromium.org, darin@apple.com, and mark.lam@apple.com) The commit-queue is continuing to process your patch.
Comment on attachment 289510 [details] Patch Attachment 289510 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2122770 New failing tests: fast/images/move-image-to-new-document.html
Created attachment 289516 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 289510 [details] Patch Clearing flags on attachment: 289510 Committed r206246: <http://trac.webkit.org/changeset/206246>
All reviewed patches have been landed. Closing bug.