Bug 39706 - Make more HTML DOM members private, especially constructors, batch 2
Summary: Make more HTML DOM members private, especially constructors, batch 2
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Adler
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-25 19:30 PDT by Darin Adler
Modified: 2010-05-28 09:28 PDT (History)
4 users (show)

See Also:


Attachments
Patch (100.74 KB, patch)
2010-05-25 19:39 PDT, Darin Adler
levin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2010-05-25 19:30:16 PDT
Make more HTML DOM members private, especially constructors, batch 2
Comment 1 Darin Adler 2010-05-25 19:39:32 PDT
Created attachment 57064 [details]
Patch
Comment 2 David Levin 2010-05-26 22:46:04 PDT
Comment on attachment 57064 [details]
Patch

WebCore/html/HTMLKeygenElement.cpp:52
 +          RefPtr<HTMLOptionElement> o = HTMLOptionElement::create(optionTag, document, this->form());
Consider adding a constructor for HTMLOptionElement which doesn't need to be passed "optionTag" as you did in other places in this patch and the previous one.



WebCore/html/HTMLOptGroupElement.cpp: 
 +      bool result = HTMLFormControlElement::insertBefore(newChild, refChild, ec, shouldLazyAttach);
I wonder if "HTMLFormControlElement::insertBefore" is used anywhere now (ditto for the other method calls that were removed here).
Comment 3 Darin Adler 2010-05-27 23:07:15 PDT
(In reply to comment #2)
> (From update of attachment 57064 [details])
> WebCore/html/HTMLKeygenElement.cpp:52
>  +          RefPtr<HTMLOptionElement> o = HTMLOptionElement::create(optionTag, document, this->form());
> Consider adding a constructor for HTMLOptionElement which doesn't need to be passed "optionTag" as you did in other places in this patch and the previous one.

I'll do that.

> WebCore/html/HTMLOptGroupElement.cpp: 
>  +      bool result = HTMLFormControlElement::insertBefore(newChild, refChild, ec, shouldLazyAttach);
> I wonder if "HTMLFormControlElement::insertBefore" is used anywhere now (ditto for the other method calls that were removed here).

I'm not sure I understand your question or suggestion here. These were unnecessary virtual function overrides that did nothing but call through to the parent class. I removed them. But the functions themselves are still used.
Comment 4 David Levin 2010-05-27 23:51:23 PDT
(In reply to comment #3)
> > WebCore/html/HTMLOptGroupElement.cpp: 
> >  +      bool result = HTMLFormControlElement::insertBefore(newChild, refChild, ec, shouldLazyAttach);
> > I wonder if "HTMLFormControlElement::insertBefore" is used anywhere now (ditto for the other method calls that were removed here).
> 
> I'm not sure I understand your question or suggestion here. These were unnecessary virtual function overrides that did nothing but call through to the parent class. I removed them. But the functions themselves are still used.

You answered it.

 I thought that maybe the functions were unused and now the functions that they called were unused.

(Sorry about taking so long to reply. I never saw your comment. When you review something, you don't get added to the cc list automatically unlike when you just leave a comment like this.)
Comment 5 Darin Adler 2010-05-28 08:34:32 PDT
(In reply to comment #4)
> (Sorry about taking so long to reply.

You took less than an hour to reply!

> I never saw your comment. When you review something, you don't get added to the cc list automatically unlike when you just leave a comment like this.)

Yes, I noticed that too. As you are aware I do a lot of reviews, so I am often affected by this. I guess we should change it.
Comment 6 Darin Adler 2010-05-28 08:39:25 PDT
Committed r60361: <http://trac.webkit.org/changeset/60361>
Comment 7 WebKit Review Bot 2010-05-28 09:28:07 PDT
http://trac.webkit.org/changeset/60361 might have broken GTK Linux 64-bit Release