Bug 67571 - window.HTMLSpanElement does not exist
Summary: window.HTMLSpanElement does not exist
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
: 12333 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-09-03 18:59 PDT by Adam Barth
Modified: 2024-01-16 08:07 PST (History)
9 users (show)

See Also:


Attachments
Patch (21.74 KB, patch)
2011-09-03 19:00 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (27.08 KB, patch)
2011-09-03 19:14 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (39.70 KB, patch)
2011-09-03 21:33 PDT, Adam Barth
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (39.56 KB, patch)
2011-09-03 22:34 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch for landing (159.79 KB, patch)
2011-09-05 17:13 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2011-09-03 18:59:20 PDT
window.HTMLSpanElement does not exist
Comment 1 Adam Barth 2011-09-03 19:00:45 PDT
Created attachment 106271 [details]
Patch
Comment 2 Collabora GTK+ EWS bot 2011-09-03 19:08:01 PDT
Comment on attachment 106271 [details]
Patch

Attachment 106271 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9589336
Comment 3 Gyuyoung Kim 2011-09-03 19:09:20 PDT
Comment on attachment 106271 [details]
Patch

Attachment 106271 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9590265
Comment 4 Early Warning System Bot 2011-09-03 19:12:53 PDT
Comment on attachment 106271 [details]
Patch

Attachment 106271 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9584961
Comment 5 Adam Barth 2011-09-03 19:14:05 PDT
Created attachment 106273 [details]
Patch
Comment 6 Adam Barth 2011-09-03 19:14:19 PDT
(It helps if I "svn add" the code.)
Comment 7 WebKit Review Bot 2011-09-03 20:55:39 PDT
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 8 Darin Adler 2011-09-03 21:00:39 PDT
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
Comment 9 Adam Barth 2011-09-03 21:02:45 PDT
(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.
Comment 10 Adam Barth 2011-09-03 21:33:10 PDT
Created attachment 106275 [details]
Patch
Comment 11 WebKit Review Bot 2011-09-03 21:45:37 PDT
Comment on attachment 106275 [details]
Patch

Attachment 106275 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9591233
Comment 12 Early Warning System Bot 2011-09-03 21:47:50 PDT
Comment on attachment 106275 [details]
Patch

Attachment 106275 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9584987
Comment 13 Gyuyoung Kim 2011-09-03 21:49:36 PDT
Comment on attachment 106275 [details]
Patch

Attachment 106275 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9591234
Comment 14 Adam Barth 2011-09-03 21:56:03 PDT
Comment on attachment 106275 [details]
Patch

Today is not my day for uploading patches!
Comment 15 Adam Barth 2011-09-03 22:34:41 PDT
Created attachment 106280 [details]
Patch
Comment 16 WebKit Review Bot 2011-09-03 23:20:22 PDT
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
Comment 17 Sam Weinig 2011-09-04 13:43:08 PDT
Any idea if there are others we are missing?
Comment 18 Adam Barth 2011-09-04 13:58:46 PDT
> Any idea if there are others we are missing?

Yeah.  There are a bunch more.  You can read them off from HTMLTagNames.in.
Comment 19 Adam Barth 2011-09-05 17:13:43 PDT
Created attachment 106366 [details]
Patch for landing
Comment 20 WebKit Review Bot 2011-09-05 18:51:54 PDT
Comment on attachment 106366 [details]
Patch for landing

Clearing flags on attachment: 106366

Committed r94545: <http://trac.webkit.org/changeset/94545>
Comment 21 WebKit Review Bot 2011-09-05 18:52:00 PDT
All reviewed patches have been landed.  Closing bug.
Comment 22 Alexey Proskuryakov 2022-12-19 20:28:57 PST
*** Bug 12333 has been marked as a duplicate of this bug. ***