Closed
Bug 522956
Opened 15 years ago
Closed 14 years ago
Clicking Larry's "More Information" button opens Page Info dialog but doesn't close Larry
Categories
(Core :: Widget: Gtk, defect)
Tracking
()
VERIFIED
FIXED
mozilla2.0b7
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: elmar.ludwig, Assigned: enndeakin)
References
Details
(Keywords: regression)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
enndeakin
:
review+
|
Details | Diff | Splinter Review |
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20091010 Minefield/3.7a1pre ID:20091010030715
When I click the "More Information" button on Larry's UI, the Page Info dialog opens but Larry doesn't close (and is, therefore, in front of the Page Info dialog). This is basically the same as bug 402415, which disappeared (or was fixed?) around the beginning of 2008.
This seems to have regressed again in the 2008-08-18 trunk build:
firefox-3.1a2pre-2008-08-17-02: works fine
firefox-3.1a2pre-2008-08-18-02: shows the bug
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-08-17+02%3A00%3A00&enddate=2008-08-18+02%3A00%3A00
Strangely, I cannot reproduce this on Firefox 3.5.3, which is also based on the 1.9.1 branch...
Comment 1•15 years ago
|
||
Indeed, this has been bugging me for months. Can we just take johnath's patch in bug 402415 to wallpaper this?
I know this affects Minefield (3.7) on 1.9.3. Does it also affect Namoroka (3.6) on 1.9.2?
blocking2.0: --- → ?
(In reply to comment #1)
> Indeed, this has been bugging me for months. Can we just take johnath's patch
> in bug 402415 to wallpaper this?
>
> I know this affects Minefield (3.7) on 1.9.3. Does it also affect Namoroka
> (3.6) on 1.9.2?
I ran across this today during 3.6rc1 testing
Comment 6•15 years ago
|
||
Neil, is this a problem with panels on Linux?
Assignee | ||
Comment 7•15 years ago
|
||
Yes, similar to bug 545429.
Comment 9•14 years ago
|
||
I just marked #540096 as dupe. Only strange thing is that the reporter claims that it works fine in Fx 3.5.7 (but not after that).
I still see this on the latest minefield nightly and think there should be a priority to finally fix this for 4.0
Comment 10•14 years ago
|
||
Blocking+, since this is pretty broken looking, and affects multiple different pieces of UI.
Assigning to Neil for now. If there's a more appropriate owner, please re-assign.
Assignee | ||
Comment 11•14 years ago
|
||
This should roll up open menus/popups upon move, resize, activate, deactivate, close, minimize and maximize on a toplevel window or dialog.
Attachment #464484 -
Flags: review?(karlt)
Comment 12•14 years ago
|
||
Comment on attachment 464484 [details] [diff] [review]
something like so
If it were solely this bug report, attachment 287401 [details] [diff] [review] seemed an appropriate solution here (rather than checking every possible event).
However, bug 545429 indicates that there is a general problem that popups are
not closed when unrelated commands are executed, so doing this in
OnContainerFocusOutEvent sounds good to me.
(I hope it will also mean that CaptureRollupEvents will no longer need to grab
pointer and keyboard when !aConsumeRollupEvent - currently it is consuming
rollup events on other apps, stealing Alt-TAB for window switch, etc. - but
that's another bug.)
I can't think why this should be necessary on
activate/OnContainerFocusInEvent, and I don't think the changes to OnWindowStateEvent are necessary as the window would lose focus when minimized or hidden, or are we opening popups when we don't have focus?
Detecting maximize seems unnecessary if rollup is going to happen on resize.
I'm not clear why rollup should also happen on move/resize but I think that
should be fine.
OnConfigureEvent is currently not detecting resizes when the top-left stays
the same, though.
Assignee | ||
Comment 13•14 years ago
|
||
(In reply to comment #12)
> I can't think why this should be necessary on
> activate/OnContainerFocusInEvent
Probably not needed, but isn't harmful. Would you prefer I removed it from focusin?
> and I don't think the changes to OnWindowStateEvent are necessary as
> the window would lose focus when minimized or hidden
I wasn't sure if minimizing removed the focus from a window on all Linux environments (on Windows, focus isn't removed and is on the minimized button)
, or are we opening popups when we don't have focus?
Popups can be opened in background windows, tooltips for instance.
> Detecting maximize seems unnecessary if rollup is going to happen on resize.
I can check for that state.
> I'm not clear why rollup should also happen on move/resize but I think that
> should be fine.
Either that or the visible popups would need to be moved relative to the new position of the window. Both Windows and Mac close popups when a popup is moved or resized.
> OnConfigureEvent is currently not detecting resizes when the top-left stays
> the same, though.
Ah, I should move the rollup check before the bounds check, or some other event is needed here?
Comment 14•14 years ago
|
||
(In reply to comment #13)
> (In reply to comment #12)
> > I can't think why this should be necessary on
> > activate/OnContainerFocusInEvent
>
> Probably not needed, but isn't harmful. Would you prefer I removed it from
> focusin?
I would prefer it is removed thanks. If it's not necessary then I'd feel
safer from (perhaps unlikely) unexpected order of focus and mouse events.
> > and I don't think the changes to OnWindowStateEvent are necessary as
> > the window would lose focus when minimized or hidden
>
> I wasn't sure if minimizing removed the focus from a window on all Linux
> environments (on Windows, focus isn't removed and is on the minimized button)
I think we should be safe there. Focus is lost when the window becomes not
viewable (according to man XSetInputFocus). (Not viewable means it or one of
its ancestors is unmapped, rather than any indication of being obscured by
other windows, etc.) GDK assumes that iconified (minimized) windows are
unmapped (and wouldn't notify us if they were mapped) and this is consistent
with ICCCM http://tronche.com/gui/x/icccm/sec-4.html#s-4.2.5
> , or are we opening popups when we don't have focus?
>
> Popups can be opened in background windows, tooltips for instance.
Ah yes, tooltips are a good example of popups without focus, but I don't think
we capture rollup events for tooltips. Tooltips should be hidden on mouse
LeaveNotify I think (though apparently that's not always happening - bug
440652).
Grabbing the pointer (as we do for capturing rollup events) from unfocused
windows would be quite nasty. If we are doing that then I think that should
be fixed (in a different way in a different bug).
> > I'm not clear why rollup should also happen on move/resize but I think that
> > should be fine.
>
> Either that or the visible popups would need to be moved relative to the new
> position of the window. Both Windows and Mac close popups when a popup is moved
> or resized.
OK, thanks. Consistency with Windows/Mac sounds good/simplest here.
> > OnConfigureEvent is currently not detecting resizes when the top-left stays
> > the same, though.
>
> Ah, I should move the rollup check before the bounds check, or some other event
> is needed here?
AFAIK moving the rollup check to before the bounds check should be fine here.
OnConfigureEvent is called "when the size, position or stacking of the
widget's window has changed". I don't actually see any events on stacking
(z-)order change on my system here, possibly because it's actually the window
manager's parent frame window that is changing order. Stacking order change
sounds a good time to rollup if these events are ever received, and if the
order of stacking and click events is suitable, so I don't know whether or not
to recommend checking for changes in bounds (size or position).
Actually the bounds check looks wrong for toplevel windows anyway, as the
aEvent.x has coords relative to the parent window but mBounds.x is root window
(screen) coords.
I'm OK if you just move rollup check to before the bounds check and we can see
how that goes.
Assignee | ||
Comment 15•14 years ago
|
||
Attachment #464484 -
Attachment is obsolete: true
Attachment #465653 -
Flags: review?(karlt)
Attachment #464484 -
Flags: review?(karlt)
Updated•14 years ago
|
Attachment #465653 -
Flags: review?(karlt) → review+
Assignee | ||
Comment 16•14 years ago
|
||
I'm continuing to investigate an odd test failure in test_largemenu.xul caused by this patch.
Assignee | ||
Comment 17•14 years ago
|
||
The problem with the test is caused because GTK or the window manager updates its position/size data directly when a window/popup is moved, but doesn't fire the configure notification events until a bit later. Thus, when trying to get the window position, we get the new coordinates before the event has occured, and think the window has moved correctly. When those events do arrive, we've already moved on to opening a popup, and it is then closed due to this fix.
Attachment #465653 -
Attachment is obsolete: true
Attachment #475626 -
Flags: review+
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Comment 18•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Component: General → Widget: Gtk
Keywords: checkin-needed
Product: Firefox → Core
QA Contact: general → gtk
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b7
Comment 19•14 years ago
|
||
Verified fixed
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100917 Firefox/4.0b7pre
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•