[webkit-dev] Moving methods from Frame to various other classes

[Maciej Stachowiak]

On Dec 25, 2008, at 2:46 PM, Holger Freyther wrote:

> Hey,
>
> there are some comments in Frame.h regarding moving functionality to  
> different
> classes and on IRC it was confirmed that the comments are old but  
> current. I
> have decided to do something about it.
>
> I have created a git branch[1] on George's server that will contain  
> the work
> in progress of the moving. I'm currently moving stuff around, it  
> will be
> followed by build fixes and speculative changes for Qt, Mac and then  
> regression
> testing on the mac. I hope to be finished with this by the start of  
> the yearly
> CCC event.

It's great that you are doing this!

> Meanwhile I would like to start some discussion on how this patch  
> should be
> put into the bugtracker and comments on the moving.
>
> I wonder if I should put each move up for separate review and then  
> land it in
> one go? Should I create a bug report for that?

It would be better to break the changes down instead of submitting as  
one big patch. Perhaps breaking things up by target class being moved  
to would be best.

> Moving comments:
>
>
> Zoom and FrameView:
> 	- Currently on history navigation (back/forward) we create a new  
> FrameView.
> When storing the Zoom information in the FrameView instead of the  
> Frame the
> Kit parts need to properly restore the Zoom Information? Is that  
> wanted?
> should we leave this functionality in the frame?

I think it would be best to leave it for now. If we had more than one  
piece of view state that persists across navigations like this, it  
might be worth making a new class to hold that state, but not for zoom  
alone in my opinion.

> Status and Chrome:
> 	- For statusBarText and defaultStatusBarText? Do we really need to  
> store the
> defaults? If yes should we do it in Chrome? Would the DOMWindow be a  
> better
> place to store them? What I have difficulties with is that the  
> information is
> set on the Chrome/Kit but that we can have a per frame default...

This is a somewhat confusing area. JS may set status bar text but  
there may also be a message set by the app. Other browsers have been  
changing to restrict or remove the ability for JS in the web page to  
set status bar text as a security measure. In my opinion we should  
review what they do and make the appropriate restrictions and  
simplifications here, separate from any refactorings.

> Editing:
> 	- I have killed Frame::removeEditingStyleFromElement and
> Frame::removeEditingStyleFromBodyElement they have been no-ops since  
> the end
> of 2006.

Removing is the best kind of moving. :-)

  - Maciej