Summary: | Make more HTML DOM members private, especially constructors, batch 2 | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Darin Adler <darin> | ||||
Component: | DOM | Assignee: | Darin Adler <darin> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, eric, levin, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Darin Adler
2010-05-25 19:30:16 PDT
Created attachment 57064 [details]
Patch
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).
(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. (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.) (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. Committed r60361: <http://trac.webkit.org/changeset/60361> http://trac.webkit.org/changeset/60361 might have broken GTK Linux 64-bit Release |