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

Ivan Timofeev timofeev.i.s at gmail.com
Wed Feb 1 09:06:38 PST 2012


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


More information about the LibreOffice mailing list