Re: [Libreoffice] [PATCH] follow-up of fdo#30788: fix showing of unnecessary empty horizontal scrollbar

Hi Caolán,

30.01.2012 21:08, Caolán McNamara пишет:
> The original code with...
> git show 7b0b5cdf source/ui/inc/scroll.hxx source/ui/uiview/scroll.cxx
> it seems that this SwScrollbar had its own SwScrollbar::Show method and
> inherits from Window which has a Window::Show method as well. The
> Window::Show isn't virtual, so it was just an unfortunately named
> wrapper method that only works if called directly.
>
> git show 5eeb54d5 then appears to replace a lot of "Show" with
> Scrollbar::Show so various code paths that used to go through the
> Wrapper method no longer do, but go direct to the baseclass Show.
>
> 02a9026b then comes along and fixes the warning that SwScrollbar::Show
> hides the Window::Show by renaming it ExtendedShow
>
> So... at the lines where we currently have ScrollBar::Show, where your
> patch adds bVisible = ..., the code used to call what is now
> "ExtendedShow" which sets bVisible = ... as well as some other stuff.

Wow, great explanation, thank you!

> At this stage it's a mangled train-wreck and e.g. changing the
> SwScrollbar::Show lines (except the one inside SwScrollbar::Show itself)
> back to ExtendedShow might trigger another pile of bugs.

Yes, that doesn't work, there is the suspicious condition inside 
ExtendedShow...
    if( (!bSet ||  !bAuto) && IsUpdateMode() && bSizeSet)
        ScrollBar::Show(bSet);

On the one hand, adding bVisible = ... works in the cases which I 
tested, on the other hand, it's impermissible to add the code without 
actual understanding how it will work. So, I'm confused...

I think the right thing to do is to look at the SwScrollbar usages and 
try to remove this bVisible insanity, if it's ever possible...

Regards,
Ivan