95348 - [FIX]BEOS - nsListBox does not display scroll bars properly, and has a poor repaint

Status: RESOLVED INVALID

(Core :: XUL, defect, P2)

Description

[Paul]

- The BeOS implementation of nsListBox does not position the scroll bars properly
- the scroll bars are not sized properly
- repaint of the listbox is very, very, very poor

Comment 1

[Paul]

I just did an update, and it seems nsListBox was removed from the BeOS
directory.  I will do a build to see if problem is fixed.

Comment 2

[Paul]

Following this comment will be a patch.  I took the nsWidgetFactory from the qt
port, and did some minor modifications.  I also changed the Makefile.in to only
compile needed code.  Everything compiled, and seems to be working, ok. 
One thing, however.  In the box I
am currently writing in, the scrollbar drawn like the one for the window. 
Though, on some bug pages, and other form pages, it seems that on list boxes, it
looks like a native BeOS scroll bar.  Anyone know where this might be coming
from?  If you go into the preferences panel, all widgets are drawn as they
should, non-native.  Though on a page, it seems as if some are native.   Now,
nsScrollBar does still have a native scroll bar within it, but not all scroll
bars are painted natively, though, some are, which is when the problem arises. 
I wonder if I can get rid of nsScrollBar to?  Oh well, something is still not
right, but at least I'm starting to learn more about mozillas internals (slowly :) )

Comment 3

[Paul]

Attachment: patch to nsWidgetFactory and Makefile.in

Comment 4

[Paul]

Attachment: zip file containing the files that were patched above

Comment 5

[Paul]

The above patch does change the nsWidgetFactory code to work the same way as the
other
ports.  The problem of nsListBox is not really what this bug is for.  I've found
that the problem
lies in nsScrollBar, since the above patch removes all other native widgets but
nsScrollBar.  I will clean up the code within the widget directory, and look
into nsScrollBar, for the next patch.  The above patch could/should be enterred
into the trunk.  (Maybe placed under a different Bug ID?)

Comment 6

[Keyser Sose]

Not sure how we missed this..trying layout..

Marking NEW.

Comment 7

[Marc Attinasi]

Note that we use native scrollbars for listboxes, and only listboxes I think.
Other scrollbars are GFX, hence the difference you see. 

Over to rods to review and/or accept the changes to the form controls.

Comment 8

[Paul]

The above patch does not fix nsListBox. In fact, the above patch removes
nsListBox from the makefile.   Instead it brings the BeOS implementation of
nsWidgetFactory up to speed with the other ports.  The problem is with native
scroll bars under BeOS.  Another bug, Bug 18895, states that all native
scrollbars should be replaced by gfx scroll bars.  Hopefully this is true, and
if it is, this bug can be ignored.  This being the case, I am working on
implementing dnd on BeOS, and the above patch can either be included then, or now.  

Comment 9

[cls]

After manually applying the Makefile.in patch and grabbing nsWidgetFactory.cpp
from the zip file and removing the nsFileWidget references,,  the patch works
and even trims off some leaks & bloat.  r=cls  I'll attach the updated version
of the patch.

Comment 10

[cls]

Attachment: Paul's patch --- one month later

Comment 11

[cls]

Comment on attachment 46851 [details] [diff] [review]
patch to nsWidgetFactory and Makefile.in

Marking obsolete

Comment 12

[Marc Attinasi]

Comment on attachment 50078 [details] [diff] [review]
Paul's patch --- one month later

sr=attinasi

Comment 13

[Paul]

Chris,  q quick note.    The latest patch posted on this page has nsSound
removed from the widgetFactory code.  This is very simple to add, but can cause
a crash with out it.  If something tries to call nsSound->beep(), mozilla
crashes.  Just look at the code in the gtk directory to get the proper values
for the array at the bottom of widgetfactory.

oh, and I'm getting close on drag n' drog, i think :)

Comment 14

[Daniel Switkin]

Ping - what's the state of this bug folks?

Comment 15

[Paul]

It looks as if patch 46852 was committed.  The nsSound stuff is missing, and
should probably have a new bug created for it.  The problem was never really
nsListBox, but scrollbars on selects.  There is another bug for implementing
selects using gfx scrollbars.  So:
1- we need to add nsSound back to nsWidgetFactory
2- either fix nsScrollbar code, or just wait for them to be implemented using gfx

I will create a new bug tonight for the nsSound stuff, and post a patch.
As for the scrollbars, we can make a new bug, and make it low priority, and a
dependency on the gfx scrollbar bug.
Once these things are done, this bug could probably be closed.  Anyone else?

Comment 16

[Sergei Dolgov]

dependency to 152085 ?

Comment 17

[Paul]

no, there is no dependency.  the code changes presented in the patches here
where checked in.  there nsSound bug you mention was a result of that checkin,
but has nothing to do with the scrollbars.

it is not worth it to fix this bug, if the scrollbar code is going to be
"thrownout" at some point in the future anyway.

Comment 18

[Sergei Dolgov]

Paul, timeless.
It seems that 99% of code in nsScrollBar.cpp does nothing.
Reason is simple - mScrollbar variable never get any pointer assigned (nobody
ever calls CreateScrollViewBeOS).
It maybe reason of buggy behaviour of scrollbars in nsListBox

Comment 19

[Sergei Dolgov]

Is there any way to force BeOS build use XBL scrollbars for nsListBox (as it is
default for main scrollbars) inspite of overal "use XBL controls" settings?

Current BeOS nsScrollBar code seems messed up, working buggy as hell, if at all,
and maybe really there is no reason to bother with it again.

Comment 20

[Paul]

Do you mean CreateBeOSView (there is no CreateScrollViewBeOS).
If so, it is called by functions in the super-class, nsWindow

Comment 21

[Sergei Dolgov]

nope. Putting debug printf in CreateBeOSView and printf in every function which
uses mScrollbar, shows that first is never called and mScrollbar is permanently
NIL - e.g on such reach for nsListBoxe-s page as
http://bugzilla.mozilla.org/query.cgi is. 
I think that some previous changes/patches created such weird situation.
(In nsWindow code CreateBeOSView is called from nsWindow::Create function.)

For scrollbar we call now only constructor from WidgetFactory

Comment 22

[Paul]

maybe you just have a bad build, because, just to prove to myself that I'm not crazy, I
tried putting a printf line in CreateBeOSView in the nsScrollBar.cpp file, and, low and
behold, the printf was called when I went to a Mozilla query page.  Think about it, it *HAS
* to be called, since that is the *ONLY* place BScrollBars are created, period.

Comment 23

[Sergei Dolgov]

yeah, you're right.
With last moment update and build finished just now it works. Though, bit
strange. CreateBeOSView for scrollbars overrides nsWindow one (though no
"virtual" in declaration in nsScrollbar.h). So is called from nsWindow:Create()
{StandardWindowCreate()} and assigns pointer to mView, and same pointer fo
mScrollbar inside function. Ok, but it seems that some other functions  should
be overriden too by nsScrollbar versions - Resize(), Move, and, maybe and i
still don't understand some details (it's unclear for me, how nsScrollbar avoids
creating BWindow and to which object it is added as child in BeOS sense.)

Problem is, Paul, that i asked local gurus on #mozilla channel about perspective
of replacement by mozilla's own scrollbars, and they said that we cannot expect
on XBL elements as default and reliable replacement in near future. 

So i'll try to fix it a little bit now

Comment 24

[Sergei Dolgov]

*** Bug 157350 has been marked as a duplicate of this bug. ***

Comment 25

[Sergei Dolgov]

*** Bug 157349 has been marked as a duplicate of this bug. ***

Comment 26

[Sergei Dolgov]

Attachment: Urgent patch

as it happened, XBL forms are broken in BeOS Mozilla 1.1 beta.
This patch allows to work with native scrollbars in ListBoxes, and makes
Mozilla for BeOS usable until fix of bug 159136

Comment 27

[Sergei Dolgov]

increasing severity - with broken XBL forms/scrollbars (bug 159136) usability of
Mozilla is low.

Comment 28

[Paul]

Sergei, in looking into the Scrollbars, have you found a way NOT to draw the
scrollbar, if it is not needed?  Many times, the horizontal scrollbar will be
drawn overtop of other information on the screen, making forms still hard to use
in mozilla.  With your patch applied, they work better, but on this page even,
the checkmark box for "Remove Selected CCs" is almost completely coverred by the
inactive horizontal scrollbar.

Comment 29

[Sergei Dolgov]

Yeah, Paul, there is the way. If i implement nsScrollbar::Resize(), i can remove
this scrollbar at all (it needs though additional check in Destroy()) - or apply
RemoveSelf for this scrollbar. Criteria is simple - aWidth==0 || aHeight == 0.

Though, it seems that there are problems outside BeOS code related to lists
formatting and sizing, dunno why those affect only BeOS - but it is real reason
for misbehaviour. Occasionally with hacking code i got sometimes proper results,
but iif i apply make clean before make - all proper drawing disappears. So it
seems depend on order in which painting and resizing was called for lists
itself, for main page view, and for those unlucky scrollbars.

Comment 30

[Paul]

Could you attach a patch which includes some of the other changes you talked
about?  Maybe a fresh set of eyes will help figure this thing out :)

Comment 31

[Sergei Dolgov]

investigating. Despite BeOS uses native scrollbars for html by default, it seems
that layout/html/base/src/nsGFXScrollFrame.cpp don't participate in this
process, but layout/html/base/src/nsScrollFrame.cpp with all those temporary
hacks inside.

Comment 32

[Asa Dotzler [:asa]]

Comment on attachment 92605 [details] [diff] [review]
Urgent patch

a=asa (on behalf of drivers) for checkin to 1.1

Comment 33

[Christian :Biesinger (don't email me, ping me on IRC)]

checked in

Comment 34

[Sergei Dolgov]

Garbage is still here.
It seems that BeZilla creates two instances of scrollbars - native BeOS one +
XBL one, which confuses all formatting.
Look at screenshot
http://www.fi.tartu.ee/~sergei_d/2ScrollBarsInBeZilla2.PNG
In red circle it can clearly see arrow from horizontal XBL-scrollbar in place of
bottom arrow of native BeOS vertical scrollbar.

Comment 35

[Sergei Dolgov]

Seems clipping bug (maybe among other problems).
Experiments for bug http://bugzilla.mozilla.org/show_bug.cgi?id=157303 affects
in several way also this unhappy low corner.

Comment 36

[Sergei Dolgov]

Current situation seems dup of
http://bugzilla.mozilla.org/show_bug.cgi?id=145992

Comment 37

[Sergei Dolgov]

"The use of native scrollbars is not supported in a xul-based mozilla (although
embedded apps may implement this differently)."
http://bugzilla.mozilla.org/show_bug.cgi?id=131719#c3
???
Why native scrollbars are enabled on page layout in BeOS ?
option "nglayout.widget.gfxscrollbars", true seems affecting only chrome, not
page layout in BeOS 

Comment 38

[Sergei Dolgov]

http://bugzilla.mozilla.org/attachment.cgi?id=10000&action=view
http://bugzilla.mozilla.org/show_bug.cgi?id=42273

Comment 39

[Sergei Dolgov]

changing component?

Comment 40

[timeless]

.

Comment 41

[Sergei Dolgov]

Patch for http://bugzilla.mozilla.org/show_bug.cgi?id=201624
obsoleted this problem.
Mozilla now uses XUL-scrollbars.