Closed Bug 36883 Opened 24 years ago Closed 24 years ago

Toolbars too far to the left, over paint their grippies, function improperly

Categories

(Core :: XUL, defect, P3)

Product:
Core
Component:
XUL
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: bugzilla, Assigned: eric)

References

Details

(Whiteboard: [dogfood+]) 

Build ID: 20000042308


In the latest nightly, the taskbar, personal toolbar and navigational toolbar 
are all too far to the left.  They are painting over their grippies, thereby 
rendering them unusable (not that they had been working correctly anyways).  

This doesn't seem hard to fix...

Can anyone else repro this?

This is present in MailNews toolbars also, though it's not as obvious as it is 
in the browser.  Will attach screenshots of all asap.
Blocks: 35181
*** Bug 36884 has been marked as a duplicate of this bug. ***
-The round toolbar buttons in Composer are only outlined - nothing inside.
(new/open/save/preview)
-The rest of the icons in Composer vanish on mouseover (and only the round
"selected" indicator" displays)

Mentioning this here since there seems to be one "allover" toolbar problem
today. Bottom toolbars don't react to mouseover or mouseclicks at all for
instance.
correction: Bottom toolbar NOW AND THEN don't react. And now and then - it does
react.
You're right; this is bigger than I thought.  I'm having trouble even getting 
the navigational buttons to function correctly -- they're not doing anything.  
Raising severity, nominating for beta2 so this gets noticed.
Severity: normal → major
Keywords: nsbeta2
It's a "Platform: All" i think (linux here)
perhaps an idea to CC pavlov, see if he recognize this? He had some possibly
related checkins over the last periode.
*** Bug 36893 has been marked as a duplicate of this bug. ***
*** Bug 36897 has been marked as a duplicate of this bug. ***
just adding some observations from the duplicates:
-Since the visual toolbar is "off" by about 10 pixels, the mouse-sensitive areas
that should correspond aren't quite corresponding today. This is in particular
causing "odd" behaviour in the small arrow and bookmark icons next to URL field,
where you have click further right than usual to get the right event to trigger.

-after having clicked back/forward/reload or stop-button, toolbar items "die"
for a while. (not sensitive to mouseover or clicks)
yup dark, you're right.  Items on the right-click context menu also "die" after 
pressing a button.

However, I just noticed that by clicking multiple times in rapid succession on 
either a toolbar item or a context-menu item, you can get them to begin 
functioning again.
Thanks to what appears to me to be a little over-enthusiastic duplicate 
marking, we now have, I think, two, maybe three bugs listed here.  What I'm 
seeing in bug 36897 has nothing to do with painting or non-responsive buttons.  
The forward, back and resolve buttons *do* respond to clicks, the surrounding 
circle "blinks" as expected.  They just don't do anything!  
ashted - I believe all of these problems regarding the toolbar can all be 
centralized and attributed to one change that someone made somewhere along the 
line.  Thus, it's silly to open multiple bugs regarding it if one person can 
fix all these problems by backing-out their changes.  Still, I'm not sure...for 
now, adding "function improperly" to the summary so this can act as a sort of 
catch-all for these problems.
Summary: Toolbars too far to the left, over paint their grippies → Toolbars too far to the left, over paint their grippies, function improperly
Understood, thanks for the explaination.  The only significant disadvantage is
that the other bugs are now marked as RESOLVED and thus don't show up when
someone new comes to report the problem.  It's just the least bit ironic that
by the consolidation, we are likely to end up with *more* bug reports :-).  
P'raps the bug helper could default to searching the descriptions and summaries
rather than just the summaries.  Anywho, here's hoping that we find that one
postulated back-out-able change :-)
I see your point.  But actually, when I'm reporting bugs I search *all* 
statuses, both open and resolved/verified.  After all, even if the same bug has 
already been reported, FIXED, and VERIFIED, it's still better to REOPEN that 
bug with a regression kw then re-reporting it as a new one.
Agreed.  On that thought I've submitted a bugzilla helper bug on the topic.  
I've no idea what the final result will be, but at least the discussion will 
happen.  I took the liberty of including you in the cc so that you'd see the 
first posting, but you're welcome to remove yourself from that position.
Adding smoketest and blocker, platform all. This has to get fixed before the 
tree reopens.

To restate the problem: The toolbar icons are all painted ~12px to the left 
of where they should be (and where the event system believes them to be). 
The Back button is painted overtop of the toolbar grippy. Clicking on various
icons does not always fire the onmousedown event (as the event system believes
the icon is in a different location).
Severity: major → blocker
Keywords: nsbeta2 → smoketest
OS: Windows 98 → All
Hardware: PC → All
reassigning to troy.  I'm not seeing this problem after backing out changes on 
4/22
Assignee: trudelle → troy
*** Bug 36990 has been marked as a duplicate of this bug. ***
*** Bug 36960 has been marked as a duplicate of this bug. ***
*** Bug 36951 has been marked as a duplicate of this bug. ***
*** Bug 36923 has been marked as a duplicate of this bug. ***
The problem I guess is in the box code. The part of the my change that caused a 
problem was just the lines I added below to CreateViewForFrame().

That code is checking for the case we have a block-level element that can 
have child frames and has 'overflow:hidden'. In that case we put a view on the 
frame so that the view manager can clip any child frames that have views.

What seems to be happening is that something in XUL is now getting a view that 
normally doesn't get a view and that causes the layout to be wrong.

#if 0
    // See if the frame is block-level and has 'overflow' set to 'hidden'. If
    // so and it can have child frames, then we need to give it a view so 
clipping
    // of any child views works correctly. Note that if it's floated it is also
    // block-level, but we can't trust that the style context 'display' value is
    // set correctly
    if (!aForce) {
      if ((display->IsBlockLevel() || display->IsFloating()) &&
          (display->mOverflow == NS_STYLE_OVERFLOW_HIDDEN)) {

        // The reason for the check of whether it can contain children is just
        // to avoid giving it a view unnecessarily
        if (::IsContainerContent(aFrame)) {
          aForce = PR_TRUE;
        }
      }
    }
#endif
Assignee: troy → evaughan
Putting on [dogfood+] radar.
Whiteboard: [dogfood+]
FYI, my changes were backed out by Scott Putterman
Yes this could pose a problem. I believe all boxes by default are overflow: 
hidden. They automatically clip their children. Is creating a view for anything 
that can clip such a good idea? What if your child was a widget? How would you 
clip it? 

One idea:
What if we added a flag to the frame's state that said whether or not it 
contained a widget or a view. That way if you were hidden you could decide 
whether you should have a clipping view or a clipping widget? 
Taking of the smoketest radar.
Severity: blocker → critical
Status: NEW → ASSIGNED
Keywords: smoketest
Blocks also clip their children that overflow, but the problem is if one of the 
child frames has a view then the view manager will handle painting of the child 
and the block frame will not be able to clip it. That's why I was giving the 
frame a view so the view manager would know to do the clipping

The assumption was that since 'overflow:hidden' is not the default most of the 
time it won't be specified and so having the view isn't a big deal

Yes, we could wait and lazily create the view if needed, but that's kind of a 
pain and potentially slow because any child views would have to be reparented to 
be a child of the lazily created view

For the time being I'm working around this problem by checking for a frame type 
of "blockFrame" and only forcing the view for 'overflow:hidden' if it's a block 
frame
My concern was that it seemed bad that XUL layout didn't work when the box had a 
view. If 'opacity' is specified that will cause the box to have a view, too, and 
the same layout problems will happen

As far as the frame state bit that indicates whether a child frame has a view, I 
agree that would be nice to have. I have wanted to add it for a while, but I 
wasn't sure whether the various frame class owners would want to be burdened 
with having to compute it and keep it correct when incremental reflows happen. 
In the case of the block code, for example, it would mean having each line cache 
whether that line had a frame with a view in it. Likewise for the various table 
frame classes
Yes your right. The UI should not have had problems when adding a view. Welcome 
to my life. :) In theory it should work. But as you found out in practice it 
doesn't always. We should definitely revisit this one. The only problem is that 
people expect boxes to clip by default. But I'm wondering if I should not do 
that. Force people to specify visiblity:hidden explicitly. Unfortunately I'm 
sure I would break a lot of code. But I just might have to do it. 

That brings us back to widgets. You can't just add a view for overflow:hidden 
you would need a clip widget also because you could have widget. That would be 
rather expensive unless we did do the flag thing on each frame. Something to 
think about.
Is his better on all platforms with second Apr 24 build?
Keywords: dogfood
fixed
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
verified fixed; linux and win32 20000425
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.