window.HTMLSpanElement does not exist
Created attachment 106271 [details] Patch
Comment on attachment 106271 [details] Patch Attachment 106271 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/9589336
Comment on attachment 106271 [details] Patch Attachment 106271 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/9590265
Comment on attachment 106271 [details] Patch Attachment 106271 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/9584961
Created attachment 106273 [details] Patch
(It helps if I "svn add" the code.)
Comment on attachment 106273 [details] Patch Attachment 106273 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9589354 New failing tests: fast/dom/gc-6.html fast/js/toString-and-valueOf-override.html editing/selection/extend-after-mouse-selection.html fast/dom/gc-7.html fast/dom/html-span-element-exists.html fast/dom/wrapper-classes.html fast/tokenizer/external-script-document-write.html fast/events/mouseclick-target-and-positioning.html
Comment on attachment 106273 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=106273&action=review > Source/WebCore/html/HTMLSpanElement.h:35 > + static PassRefPtr<HTMLSpanElement> create(Document*); While it would be nice to have this function if we ever wanted to create a span element in general purpose code, I doubt that will come up. I’d suggest leaving it out. > Source/WebCore/html/HTMLSpanElement.h:38 > +protected: I don’t think we expect to derive classes from this one, so I suggest private rather than public. > Source/WebCore/html/HTMLSpanElement.idl:2 > + * Copyright (C) 2006, 2010 Apple Inc. All rights reserved. Probably the wrong copyright here. > LayoutTests/ChangeLog:11 > + * fast/dom/html-span-element-exists-expected.txt: Added. > + * fast/dom/html-span-element-exists.html: Added. While it’s OK to add a new test, I think there are existing tests that cover all the different element types, it would be better to add to those rather than adding a whole new test just for the span element. Specifically, fast/dom/wrapper-classes.html is about this very subject. In case that test does not have sufficient coverage, things like the constructor and prototype should be tested along with the other DOM constructors and prototypes. fast/dom/wrapper-classes.html will start failing so I am going to say review- on this patch because I presume that means we haven’t succeeded at running the tests with this patch yet
(In reply to comment #8) > (From update of attachment 106273 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=106273&action=review > > > Source/WebCore/html/HTMLSpanElement.idl:2 > > + * Copyright (C) 2006, 2010 Apple Inc. All rights reserved. > > Probably the wrong copyright here. I wasn't sure what to do about the copyright block here. I based it on HTMLDivElement.idl (which is where this copyright block comes from), but it's also so simple that it seems like it should get a fresh block. > > LayoutTests/ChangeLog:11 > > + * fast/dom/html-span-element-exists-expected.txt: Added. > > + * fast/dom/html-span-element-exists.html: Added. > > While it’s OK to add a new test, I think there are existing tests that cover all the different element types, it would be better to add to those rather than adding a whole new test just for the span element. Specifically, fast/dom/wrapper-classes.html is about this very subject. In case that test does not have sufficient coverage, things like the constructor and prototype should be tested along with the other DOM constructors and prototypes. > > fast/dom/wrapper-classes.html will start failing so I am going to say review- on this patch because I presume that means we haven’t succeeded at running the tests with this patch yet Will do. Thanks for the review.
Created attachment 106275 [details] Patch
Comment on attachment 106275 [details] Patch Attachment 106275 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9591233
Comment on attachment 106275 [details] Patch Attachment 106275 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/9584987
Comment on attachment 106275 [details] Patch Attachment 106275 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/9591234
Comment on attachment 106275 [details] Patch Today is not my day for uploading patches!
Created attachment 106280 [details] Patch
Comment on attachment 106280 [details] Patch Attachment 106280 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9591251 New failing tests: fast/js/toString-and-valueOf-override.html fast/dom/wrapper-classes.html
Any idea if there are others we are missing?
> Any idea if there are others we are missing? Yeah. There are a bunch more. You can read them off from HTMLTagNames.in.
Created attachment 106366 [details] Patch for landing
Comment on attachment 106366 [details] Patch for landing Clearing flags on attachment: 106366 Committed r94545: <http://trac.webkit.org/changeset/94545>
All reviewed patches have been landed. Closing bug.
*** Bug 12333 has been marked as a duplicate of this bug. ***