Bug 148290 - Drop NodeListBase class
Summary: Drop NodeListBase class
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-20 22:02 PDT by Chris Dumez
Modified: 2015-08-21 02:54 PDT (History)
4 users (show)

See Also:

Attachments
Patch (11.30 KB, patch)
2015-08-20 22:05 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2015-08-20 22:02:03 PDT 
Drop NodeListBase class and have HTMLCollection subclass NodeList directly. There is no reason we need another base class.
Comment 1 Chris Dumez 2015-08-20 22:05:08 PDT 
Created attachment 259584 [details]
Patch
Comment 2 Ryosuke Niwa 2015-08-21 00:41:55 PDT 
Comment on attachment 259584 [details]
Patch

Funny, I've done exactly this in the past until Antti refactored it.
Comment 3 WebKit Commit Bot 2015-08-21 01:28:18 PDT 
Comment on attachment 259584 [details]
Patch

Clearing flags on attachment: 259584

Committed r188753: <http://trac.webkit.org/changeset/188753>
Comment 4 WebKit Commit Bot 2015-08-21 01:28:22 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 5 Antti Koivisto 2015-08-21 02:51:55 PDT 
Comment on attachment 259584 [details]
Patch

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

> Source/WebCore/html/HTMLCollection.h:62
> -class HTMLCollection : public NodeListBase, public ScriptWrappable {
> +// HTMLCollection subclasses NodeList to maintain legacy ObjC API compatibility.
> +class HTMLCollection : public NodeList {

It is somewhat confusing that HTMLCollection is now a subclass of NodeList in C++ but not in IDL. The pure virtual base might have been a better way to deal with the the legacy issue mentioned in the comment.
Comment 6 Antti Koivisto 2015-08-21 02:54:58 PDT 
NodeListBase was a bad name though, something like LegacyObjCAPICollection would have been more informative.