Re: commit (HEAD): bidi fixes and toolbar

From: Tomas Frydrych (tomas@frydrych.uklinux.net)
Date: Sat Jan 25 2003 - 15:54:30 EST

  • Next message: Dom Lachowicz: "Commit: some PO patches from Raphael"

    Hi Jeremy,

    sorry if the comments came across harsh; I have spent several
    hours today trying to work out why the toolbar would not respond to
    notification messages from the view. At the end I discovered that in
    the win32 frame there vere some vectors declared that that had the
    same names as vectors the xap impl class but got never used, and
    consequently the win32 frame was under the impression it did not
    have any toolbars. So I have deleted those vectors, at which point it
    broke badly, because the frame cannot access the members of the
    impl, since it is not its base class. That's why I had to move the
    code for _bindToolbars to the impl, but that was not enought -- I
    shuffled stuff around immitating the Unix frame, until I got it work.

    I have to say honestly that I do not see what we gained by having
    Frame and FrameImpl classes, except a maintanance nightmare; I
    see no clear conceptual distinction between the two, and trying to
    find out what is done from where is difficult (not just in the win32),
    but perhaps I am just ignorant :).

    Tomas

    > Note: toolbar stuff is inprogress (until today I haven't had a
    > chance to work on it) so it has yet to be split (i.e. unaltered), and
    > this is a known issue. I also have other changes in my local tree,
    > where there is almost nothing to ap_Win32Frame, so I'm wondering
    > (ignoring anything related to toolbars) what implementation stuff is
    > there in ap_Win32Frame that should be in ap_Win32FrameImpl? [I am
    > going to ignore the funny issue of ap_Win32FrameImpl is the
    > implementation of ap_Win32Frame which is the Windows implementation of
    > ap_Frame.]
    >
    > Let's see, methods in ap_Win32Frame that don't directly forward to
    > ap_Win32FrameImpl - the construction/destruction/initialization stuff,
    > I don't see anything here that should be put into the impl - toolbar
    > functions, covered above, being done as I write this - bindToolbars,
    > already moved in my local tree and I see now you did this in cvs
    > version as well - toggleStatusBar, it calls some FrameData stuff,
    > which I thought was silly to have the impl do (why FrameData still
    > exists now that there is an impl class I'm not sure, but more work
    > than I wanted to do) and then calls into the impl class to update the
    > container window. - get/setZoomPercentage, calls super class func or
    > uses FrameData - createViewGraphics, calls impl to create, sets the
    > zoom, returns - setViewFocus, empty - createScrollBarListener, clearly
    > marked as INPROGRESS - replaceView, override to add in code because of
    > a logic flaw somewhere - scrollFuncX/Y, gets the Frame then calls impl
    > function - that's it
    >
    > I realize you don't have access to my local changes, but I
    > would appreciate it if sometime in the next few weeks after
    > I commit them if you would let me know what you think should
    > still be move to the impl class.
    >
    > ...
    > TF> Tomas
    > TF>
    >
    > Jeremy Davis
    > jeremyd@computer.org
    >
    >
    >



    This archive was generated by hypermail 2.1.4 : Sat Jan 25 2003 - 15:59:27 EST