Summary: | Frame Refactor: Move isFrameSet to Document | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Holger Freyther <zecke> | ||||||
Component: | WebKit Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | ||||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Holger Freyther
2009-01-20 03:52:08 PST
Created attachment 26856 [details]
Carry out the move
I think the added null check (if correct) looks ugly and it would be a case for leaving isFrameSet in Frame. Test cases pass on the mac.
Comment on attachment 26856 [details] Carry out the move > +bool Document::isFrameSet() const > +{ > + if (!isHTMLDocument()) > + return false; This should be a virtual function, then, and be overridden in HTMLDocument. That wasn't possible before when this was a function on Frame. I'll say r=me, but it would be better to do it that way Created attachment 27209 [details]
Make isFrameSet virtual
Take Darin's comment into account and make isFrameSet virtual. Reimplement it in HTMLDocument (this patch is not tested).
Comment on attachment 26856 [details] Carry out the move Landed in r40443, leaving this bug open due the new patch. Comment on attachment 27209 [details] Make isFrameSet virtual r=me > +bool HTMLDocument::isFrameSet() const > +{ > + HTMLElement *bodyElement = body(); * should be on the other side. Closing it. The style fix was included. |