Closed Bug 191265 Opened 22 years ago Closed 19 years ago

Native scrollbars draw when hidden, and don't get clipped correctly

Categories

(Core :: Web Painting, defect)

Product:
Core
Component:
Web Painting
Platform:
PowerPC
macOS
Type:
defect
Priority:
Not set
Severity:
major

Tracking

()

Status:
RESOLVED DUPLICATE of bug 187435
Milestone:
mozilla1.4alpha

People

(Reporter: justdave, Assigned: bryner)

References

(
URL
)

Details

Attachments

(8 files, 4 obsolete files)

screenshot from Mozilla 2003011203 OS X
96.94 KB, image/jpeg
Details
screenshot from Netscape 7.01 OS X
117.88 KB, image/jpeg
Details
Source code - mailing address page
139.34 KB, text/html
Details
Source code - Introduction page
133.13 KB, text/html
Details
Source package (save as complete page, tar.gz)
66.82 KB, application/octet-stream
Details
oops ... real testcase
283 bytes, text/html
Details
screenshot of cocoa scrollbar resizing experiment
30.50 KB, application/pdf
Details
patch (carbon only)
16.12 KB, patch
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.3b) Gecko/20030112
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.3b) Gecko/20030112

The TurboTax for the Web software works MUCH better than it did last year (I had
to run it in MSIE last year because it wouldn't even render in Mozilla),
however, this year it has a floating scrollbar in the middle of the work area on
EVERY page (once you start the program).

I'm using TurboTax for the Web Basic version

Reproducible: Always

Steps to Reproduce:
1. visit the linked URL
2. Start a new return
3. once you've created your ID, it loads the workspace area, which takes a
minute or two.
Actual Results:  
Once the workspace area loads, you have a floating scrollbar in the middle of
your workspace, which is blocking a lot of the information you're supposed to be
verifying.

Expected Results:  
No floating scrollbar.

This is a major site this time of year, LOTs of people will be using it.
(setting severity accordingly :)

I'll attach a screenshot in a moment.
I was told on #mozilla that I needed to CC SusieW on this because she'd think it
was important.
Duplicated in 2003012213 OS X trunk

works fine in MSIE 5.2.2 of course.
It says "last name" under the toolbar in the screenshot. Can I mark it fixed?
Just kidding. I'll try to contact them.
Assignee: aruner → susiew
TurboTax deluxe online and TurboTax basic works for me (no problem) on Win2K,
with 11/20 branch (Netscape 7.01)

Can you test this on an old Mac build to see if the problem exists?
Summary: TurboTax for the Web displays a floating scrollbar in the middle of the work area → TurboTax for the Web (Mac) - floating scrollbar in the middle of the work area
Duplicated in Netscape 7.01 OS X
Attachment #113079 - Attachment description: screenshot → screenshot from Mozilla 2003011203 OS X
cannot duplicate this in Mozilla 2002101612 Mac OS 9.2, so it appears to be OS X
specific....  UI bug in Gecko on OS X?
cc: some mac folks
I just filed my taxes using this site with Mozilla 1.0.1 on Linux and didn't see
this issue, so either it doesn't apply to Linux or it doesn't apply to the 1.0
branch (or both).
Can someone get the source from that page?
(a minimal testcase would be great; I can do it if someone can grab the source)
What does it look like on Windows/Linux?
Maybe the native theme code needs to check some visibility flags or something?
Windows 2000 - WFM, no problems

-tested with 2002121215 mozilla
-20021120 commercial branch
previous attachment was the source as downloaded for the main page.  this
attachment is the "save as complete page" option with all the supporting files
in the tarball.

Interestingly enough, if I load the saved page offline I don't get the floating
scrollbar, but I do on the live site...
FYI: Same thing happens on Chimera 2003020407.
This looks to me like one of the "expected" problems with using native
scrollbars in Gecko - z-index is not respected, so the scrollbar can paint
through from any layer.

If we want to fix this, we're going to have to stop the native scrollbar from
painting.  Pinkerton, any thoughts?

Where's the native scrollbar coming from?

Bug 126263 has a potentially promising fix that gets rid of the use of native
scrollbars on <select>. AFAIK we don't use native scrollbars anywhere else in
Mozilla proper.
except on mac we don't want no funky xul scrollbars, so we use native ones ;)

hyatt told me how they did this for safari. It's complicated and requires us to
channel all scrollbar events (painting, clicking, etc) back through gecko and
then to the scrollbar.
Gah. Can you not use nsNativeThemeMac and related tricks?
Please change this bug to the appropriate browser/component and don't cc me. Thanks!
no, not for scrollbars. the apis don't do what we need for nsITheme. trust me,
been there, done that. wrote the screenplay, saw the movie.
You can't even persuade a hidden/offscreen scrollbar to paint into a pixmap,
like GTK does?

How do you do printing?

We may be able to hack around some simple 'z-index' cases, but basic CSS
features like compositing translucent and transparent content on top of
scrollbars, and supporting 'opacity' on elements with scrollbars, just can't be
done unless we can draw scrollbars into our offscreen buffers.
Well, obviously this looks like a Browser bug and not Evangelism...  :)  I
jumped the gun on calling it Evangelism because of my bad experience with them
last year (which was definitely Evangelism :) and assumed it would be the same
problem again.

Moving per Susie's request.  I'm probably screwing up the component, someone
please reassign as necessary.
Assignee: susiew → roc+moz
Component: US Ecommerce → Layout: View Rendering
Product: Tech Evangelism → Browser
QA Contact: bclary → ian
Version: unspecified → Trunk
roc: there were numerous problems with nsIThemed scrollbars on mac.  I believe
most of them stemmed from the need to draw the entire scrollbar as a unit
whenever any part of it needed to be redrawn.
Yeah, I'm looking at bug 121440 where pink switched from nsITheme to native
scrollbars.

I wish I didn't have to bring the bad news, but I think that was a wrong turn.
I do some cocoa programming on OS X, and when a view that contains a scrollbar
gets overlapped by another view (when *everything* is drawn natively) the hidden
part of the scrollbar doesn't draw.  Is there a way to fake the OS into thinking
part of it is covered by something when you tell it to draw?
That might solve this particular problem. But a harder problem is, e.g., when
text is drawn over the scrollbar.
the real way to fix this is to pipe all drawing and event calls on the scrollbar
back through gecko so that it can go through the view manager for clipping and
hit testing. it's possible, just a lot of work.
Attached patch text over scrollbars (obsolete) — — Splinter Review 
This is the sort of thing that I doubt can be done with native scrollbars.
I believe that Safari and KHTML do not handle this testcase correctly, BTW.
Anyone who writes a web page that does that deserves to have problems.
Scrollbars are user interface elements; users are exepected to interact with
them, and have some mental/visual model of how they affect other areas of the
screen. Standards be damned! :-)

I really think that we only need to consider layouts that maintain interface
consistency (nestable scrolling frames/divs etc, hidden elements with scrollbars).
I understand the appeal of that position but I don't think it's tenable. For one
thing, you would need a clear statement of exactly what we will support and what
we won't support. Note that this bug differs from my testcase only in that the
element that covers the scrollbars covers them entirely and is opaque. So unless
you're OK with marking this bug WONTFIX, you'll need to talk about whether
elements covering scrollbars are opaque or not, what sort of overlap is allowed
(is partial overlap OK? What if the element is opaque but has rounded borders?)
You'll need to disallow other things that won't work right, such as
'-moz-opacity' (and CSS3 'opacity') on elements which could have scroll bars.
And when you're done, I guarantee that the Web standards people won't be happy,
and neither will Web authors.

Also remember that all this stuff currently works fine with Mozilla on Windows
and GTK (except for scrollbars in <select>, but we're working on that) and on IE
for Windows*, so authors will just ignore your limitations.

* I'm not completely sure, but I believe so. Someone can easily test it.
Regarding Comment #31, the test case does indeed work correctly in Safari.
Although Safari does not yet support overflow:scroll, it is worth noting that all of the native widgets used in Safari respect z-index completely.  This includes native form controls, iframes, and scrollbars.  They paint exactly when the layout engine tells them to paint and are clipped properly by the layout engine.  They also obey event handling rules properly.This is actually quite easy to do with Cocoa NSViews.  You can even go inspect how it was done in Safaris source code.
If you want another testcase for this, visit
http://landfill.bugzilla.org/bugzilla-tip/query.cgi using Mozilla, and click the
"Give me some help" link at the top of the page.  That'll reload the page with
some Javascript-enabled tooltips when you mouse-over things.  Note that any
tooltips that happen to coincide with a scrollbar on a select box wind up
getting buried under the scrollbar.  Not sure if that's the same issue or not,
but it sounds related.
Ah, the report I read was about Konqueror, not Safari. Sorry about that.

If NSViews are so flexible, then can't we fake up a scrollbar for nsITheme to
paint with?
NSViews are still "widgets" with positions relative to their parent NSViews, 
etc.  I don't think you could easily fake that up with nsITheme, since the 
widgets need to have their positions updated.

Seems like you want to associate a widget with the scrollbar frame, and then 
have the scrollbar frame tell the widget when to paint and whether it can get 
an event.  

That's how we do it in Safari at least.  The widget gets moved automatically 
when the frame moves, and it refuses to handle events or paint unless it gets 
a call through the API used by the frame to communicate with the widget.

This approach has worked for us for all Cocoa native widgets (even text fields 
and their blinking carets and focus rings).



Hmm. So you can control *when* it paints, but you can't get it to paint into an
offscreen buffer?

I guess I'm thinking of the GTK approach, where we have a single offscreen
control, and we can tell it to paint into an arbitrary rendering context, which
lets us draw one anywhere we want. And we set the desired state by passing flags
or poking it with events.
That's not quite what we do for gtk.  We have a native scrollbar widget, sure
enough, but we don't "tell it to paint".  We basically just use it to create a
GtkStyle.  We actually paint the parts of the scrollbar by hand, using that
GtkStyle.

Compare that to mac, where you _can't_ paint just part of a scrollbar.  You have
to draw the entire scrollbar every time you need to repaint part of it.  Not
only is that quite slow, it means you need to go figure out the correct state
information for all parts of the scrollbar.  i.e. to repaint an arrow, I need to
go figure out the slider position and range.  The API really isn't suited to the
way we do scrollbars in XUL.

So, let's suppose that we want to make the native scrollbars smart about when
they paint. If we could prevent the native widget from automatically repainting
itself when it's invalidated, can we then cause nsIWidget::Paint() to be called
for the scrollbar when the view system determines that it needs to paint?

> can we then cause nsIWidget::Paint() to be called
> for the scrollbar when the view system determines that it needs to paint?

Sure, we can arrange that, but you'll have to paint into the rendering context
we provide. Otherwise clipping and 'opacity' and printing, at least, won't work.
No, I want the scrollbar to be painting directly to the screen, and clipping to
be accomplished by restricting the paint rect(s) to the area(s) where the
scrollbar should be topmost.

As for printing, why do we need to print scrollbars?
> why do we need to print scrollbars?

I thought since print preview shows scrollbars on 'overflow:scroll' and (when
necessary) 'overflow:auto' elements, then we printed the scrollbars too. But now
I see that we don't print scrollbars, and therefore the fact that they're shown
in print preview is a bug. It would be nice to visually indicate that content is
overflowing, and scrollbars would be the least-surprise way to do that, but I
agree there's no compelling need to print them.

> I want the scrollbar to be painting directly to the screen

OK ... The reason I want to get the scrollbar into an offscreen pixmap is that
then I know for sure we can do whatever rendering transformations we want now
and in the future. We can clip arbitrarily, we can scale, we can do translucent
compositing, we can do absolutely anything.

Now, if we *can't* get the scrollbar into an offscreen pixmap, and we can't even
control the state of the screen rendering context that the scrollbar renders
into, then we have to get Cocoa to do all the rendering tricks that we need.
Here's a list:
-- Double-buffering. Currently we do this by painting into an offscreen pixmap.
I seem to recall that Quartz has native double buffering ability, so we'll have
to modify the GFX interfaces to expose that, and then use it.
-- Clipping. Currently we only need to clip to rectangles, so passing a
rect-to-be-painted will suffice (providing that the rect is honored exactly, and
is not just treated as advisory).
-- Compositing of translucent content onto areas containing scrollbars.
Currently we do this by manipulating offscreen pixmaps in cross-platform code.
This will have to moved to platform-specific GFX so we can get Quartz to do the
compositing directly.
-- Partial 'opacity' for content which contains scrollbars. This is also done
using offscreen pixmaps in cross-platform code. I don't see how this can be done
at all if we can't control the state of the rendering context for the scrollbar.

Actually I'm all in favour of changing our offscreen-pixmap-based functionality
into explicit GFX APIs for double-buffering and group opacity and so on, so we
can take advantage of the platform support for the above features which is
emerging. That's something I've wanted to do for a while. But that future GFX
API should promise that the state of the current rendering context actually gets
used to render everything, including scrollbars.

By the way, looking forward there are potential features that I'd like to remain
possible:
-- Zooming. (Currently zooming actually does work on GTK and Win32: try going to
a page with scrollbars in it (say, the Bugzilla query page). Do "Page Setup",
set the scale factor to, say, 10%. Now go to "Print Preview". Check out the tiny
scrollbars. Now click on them. They work!!!) If we can't at least control the
state of the rendering context Cocoa scrollbars draw into, then zooming will be
impossible.
-- SVG <foreignobject>. This is the nightmare scenario which combines all of the
above and more. Non-rectangular clipping, partial opacity, affine
transformations, convolution filters, etc. (Yet we can do all this if we can
just get the scrollbar into an offscreen pixmap!)
I'd be inclined to support what we need for CSS 1&2 (i.e. actual standards),
even if that makes it harder to support things like opacity and SVG, which
either aren't part of a standard, or just aren't widely used enough to justify
massive amounts of Gecko work.

I don't think I quite understand what you want to add to gfx in terms of double
buffering.  We already set double buffering to off in the view manager on OS X,
based on a value returned from nsRenderingContextMac::UseBackBuffer().

re: compositing, opacity, and zooming, I'm pretty sure Cocoa can't do any of these.

So what would we have to change to clip via paint rects?

> I'd be inclined to support what we need for CSS 1&2 (i.e. actual standards),
> even if that makes it harder to support things like opacity and SVG, which
> either aren't part of a standard, or just aren't widely used enough to justify
> massive amounts of Gecko work.

I'm fine with you implementing native scrollbars "the Safari way", since I think
it can be done without disrupting our existing infrastructure. I'm just pointing
out that it's not futureproof and it's not even completely adequate today. (I do
get bugs filed against me about '-moz-opacity' misbehaving.) But I agree it's
way better than what we're currently doing.

> I don't think I quite understand what you want to add to gfx in terms of
> double buffering.

I want to move the crossplatform code that's currently in the view manager down
to GFX, to simplify the view manager and to ensure that platforms with native
double-buffering don't need to ship the code.

> So what would we have to change to clip via paint rects?

Very little. We already clip the damage rect as necessary, so you can just use
the damage rect as the paint rect. I think you just need to make your frame
Paint() method call down to the widget.
it's much more than just not painting via a damage rect, because we also have to
not allow clicks on the scrollbar that is overlapped by other content. as a
result we have to pass clicks to scrollbars back through gecko as well (see
comments 18 and 28).
OK, so send your scrollbar an event when we call HandleEvent on the frame.
>re: compositing, opacity, and zooming, I'm pretty sure Cocoa can't do any of
>these.

It can.  I've done all three in my own apps.  (Though I don't know what hoops
you'd have to jump through to get to those APIs from Gecko - I've never touched
Mozilla source)  If you want an overtly obvious example, just look at the way
icons are handled in the dock :)

If you're drawing into an NSView object, changing the bounds rect without
changing the frame rect will zoom your contents to make the bounds you defined
occupy the frame rect, thus you can zoom in on something by making your bounds
rect smaller.  You can make the entire view bigger by increasing the frame, but
since changing the frame automatically adjusts the bounds, you'd have to go back
and change the bounds back to what it previously was to get the contents of the
frame to increase in size with the frame.

Opacity is hard to miss.  Almost any drawing operation you do wants an alpha
value to go with the color :-)  Guess what it's used for? :)

Compositing is a CoreGraphics (Quartz) function, but it's not too hard to do if
you're already doing Quartz stuff (not sure how you could draw some of the
things you're drawing currently if you weren't).
The question is whether we can do zooming, translucency and group compositing
for Cocoa *widgets*, where we can't modify the painting code. Show me a
screenshot of a zoomed Cocoa scrollbar and then I'll believe :-).

It seems to me that for our sake and for Safari's future, Hyatt might want to
chat with the right Apple engineers to get a solution worked out for a future
Cocoa rev. It seems that if we can get Cocoa widgets to render into an arbitrary
graphics context, that would be enough. In the meantime we can do paint and
event callbacks to the widget like Hyatt suggested.
hmm, you appear to be right (at least for scrollbars).  I tried it out just for
fun, and although the scrollbar itself resizes just fine (and the hit areas and
so forth expand and shrink just like you'd expect them to), the individual
pieces of the scrollbar (the arrow buttons and the knob) ignore the
magnification settings and draw at their normal size inside that space, which
makes it look really ridiculous. (I'll attach a screenshot momentarily)
We have to fix this for both Cocoa and Carbon widgets.
So, we could do something like this.  What I've done here is cause OS scrollbar
draws to generate NS_PAINT_EVENTs, which causes Gecko to paint things in the
right order so that scrollbars can be drawn on top of.

Unfortunately, this causes a lot of flickering of the scrollbar when you're
scrolling with it.  I'm posting the patch here in hopes that someone can help
me figure out why.
Attached patch patch to fix flickering (obsolete) — — Splinter Review 
The problem is that the scrollbar was set to have a transparent background, so
the parent frames' backgrounds were getting painted, then the paints could be
flushed prior to the native scrollbar being painted.  This patch sets a
background-color on the scrollbar so that it's treated as opaque.
Painting can get flushed *during* a paint operation, defeating double-buffering?
That sounds like it could burn us in some other contexts.
Attached patch patch for carbon and cocoa (obsolete) — — Splinter Review 
cleaned-up patch for carbon + patch for cocoa.

note that this currently only addresses painting.  events such as clicks are
still able to go to the scrollbar even when it's not visible.
Attachment #115127 - Attachment is obsolete: true
Attachment #115296 - Attachment is obsolete: true
Comment on attachment 115447 [details] [diff] [review]
patch for carbon and cocoa

Looking for some feedback on this.  I'll probably fix the event handling too
before landing this, but I wanted to see what you guys thought about the
general approach.
Attachment #115447 - Flags: superreview?(sfraser)
Attachment #115447 - Flags: review?(pinkerton)
re: roc's comment - I didn't investigate this a lot because I found a better
fix, but what appeared to be happening is that calling ::EventAvail() from
control_key_down() (which is debug-only code in widget/src/mac/nsWindow.cpp)
caused pending drawing operations to happen.  I've got no clue why.

Yeah, that EventAvail() is evil; we just need to get GetKeyModifiers or
somesuch. It causes painting to happen because it does run loop stuff.
Comment on attachment 115447 [details] [diff] [review]
patch for carbon and cocoa

>
+NS_IMETHODIMP
>+nsNativeScrollbarFrame::Paint(nsIPresContext*      aPresContext,
>+                              nsIRenderingContext& aRenderingContext,
>+                              const nsRect&        aDirtyRect,
>+                              nsFramePaintLayer    aWhichLayer,
>+                              PRUint32             aFlags = 0)

Remove the default value from the implementation. I'm surprised this built; it
would break CodeWarrior.

I'd like to see a comment to say why this view is required, and where it's
used.

>+  NSQuickDrawView* mQDView;

>+- (void)drawRect:(NSRect)rect
>+{
>+  if (mPaintingEnabled)
>+    [super drawRect:rect];
>+  else {
>+    nsRect r;
>+    r.x = NS_STATIC_CAST(nscoord, rect.origin.x);
>+    r.y = NS_STATIC_CAST(nscoord, rect.origin.y);
>+    r.width = NS_STATIC_CAST(nscoord, rect.size.width);
>+    r.height = NS_STATIC_CAST(nscoord, rect.size.height);
>+
>+    nsCOMPtr<nsIRenderingContext> rc(dont_AddRef(mGeckoChild->GetRenderingContext()));
>+    mGeckoChild->UpdateWidget(r, rc);
>+  }
>+}

This needs a lengthy comment to describe how this stuff works.

>@@ -95,6 +97,7 @@
>   PRUint32          mLineIncrement;
>   PRBool            mMouseDownInScroll;
>   ControlPartCode   mClickedPartCode;
>+  PRBool            mInPaint;

Move it up next to the other PRBool, and make them packed.
Attachment #115447 - Flags: superreview?(sfraser) → superreview-
taking.
Assignee: roc+moz → bryner
Target Milestone: --- → mozilla1.4alpha
*** Bug 232470 has been marked as a duplicate of this bug. ***
*** Bug 246810 has been marked as a duplicate of this bug. ***
Flags: blocking-aviary1.0mac?
Flags: blocking-aviary1.0mac?
Attachment #115447 - Flags: review?(pinkerton)
*** Bug 286128 has been marked as a duplicate of this bug. ***
Paint: Basically the same as bryner's patch, except that I added a visibility
check in |nsNativeScrollbarFrame::Paint()|, so that scrollbars aren't drawn
when the frame isn't visible (bug 187435).

Events: First, I had to handle |FindWidgetHit()|, which looks for the lowest
level child window to forward mouse events to.	In our case, since
|nsNativeScrollbar| is a window, that would always get the mouse events, even
if it is underneath a DIV (as in the testcase).  So I overrode
|FindWidgetHit()| in |nsNativeScrollbar| to have it always return null.  This
makes mouse events go to the base window, where the view manager will
eventually forward the events to the |nsNativeScrollbarFrame|.	There, I just
had it call |nsNativeScrollbar::HandleMouseEvents|.  Works pretty well.

I have only one remaining issue.  In the testcase, if I click on bottom arrow
of the vertical scrollbar, this makes the scrollbar paint above the div.  It
repaints properly once I move the mouse.  Any ideas on how to fix that problem?
Attachment #115447 - Attachment is obsolete: true
(In reply to comment #53)
Even with my latest patch, I still see some flickering related to scrollbars. 
You can see this at
https://bugzilla.mozilla.org/query.cgi?help=1&format=advanced.  Scrolling that
page, or just hovering over some of the select elements can cause much flickering.

It seems to me that this flickering is caused by the |Invalidate()| call in
|nsNativeScrollbar::Paint()|.  Seems to be causing excessive redraws.  I changed
|nsNativeScrollbar::Paint()| to directly call |nsMacControl::OnPaint()|, and had
|nsNativeScrollbar::OnPaint()| always return TRUE, and this seemed to fix the
flickering, although now the scrollbars sometimes draw in odd places.
*** Bug 287385 has been marked as a duplicate of this bug. ***
Summary: TurboTax for the Web (Mac) - floating scrollbar in the middle of the work area → Native scrollbars draw when hidden, and don't get clipped correctly

*** This bug has been marked as a duplicate of 187435 ***
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → DUPLICATE
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: