Closed Bug 526941 Opened 15 years ago Closed 15 years ago

Panels with noautohide="true" not taking focus

Categories

(Core :: Widget: Gtk, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b1

People

(Reporter: enndeakin, Assigned: karlt)

References

()

Details

Attachments

(2 files, 6 obsolete files)

Attached file testcase
This testcase shows two buttons that open panels when pressed. The first panel, for the button 'Panel Button', works OK. The second panel, for the button 'Panel button noautohide' doesn't focus properly. Clicking on the panel doesn't focus the elements inside it. The difference is that the former is created with: gtk_window_new(GTK_WINDOW_TOPLEVEL) whereas the latter is created with: gtk_window_new(GTK_WINDOW_POPUP) The former is created this way to allow it to hover only above its parent. (gtk_window_set_transient_for() is also called but seems to have no effect). The problem is that the popup (a widget of type eWindowType_popup) is receiving the focus-in signal and NS_ACTIVATE is firing on the popup. Instead, it should fire on the toplevel window. Focus works properly if the titlebar of the parent window is then clicked.
With my local build based on 2d504f6e2ede, I seem to get the same behavior with 'Panel button noautohide' as with 'Panel Button'. That is clicking on elements in either panel focuses them. BTW, GTK thinks of both gtk_window_new(GTK_WINDOW_POPUP) and gtk_window_new(GTK_WINDOW_TOPLEVEL) as toplevels, so I wonder what might be causing the difference you are seeing.
Oh, I should mention that you need to load the test in a chrome window to see the behaviour, as noautohide is a privileged only feature. Changing to GTK_WINDOW_POPUP from GTK_WINDOW_TOPLEVEL makes it work, but then the popup floats above all windows, including those of other applications.
Summary: Panels with noautohide="true" not taking focusing → Panels with noautohide="true" not taking focus
This patch just calls SetFocus on the parent of the popup when the mouse button is pressed. Doesn't handle child frames but the patch seems to work ok for popups.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Attachment #445756 - Flags: review?
Attachment #445756 - Flags: review? → review?(karlt)
The main difference between GTK_WINDOW_POPUP and GTK_WINDOW_TOPLEVEL is that GTK_WINDOW_POPUP indicates use of the override-redirect Window attribute. http://tronche.com/gui/x/icccm/sec-4.html#s-4.1.10 "If the window will be visible for a very short time and should not be decorated at all, the client can set override-redirect on the window. In general, this should be done only if the pointer is grabbed while the window is mapped. The window manager will never interfere with these windows, which should be used with caution." The window manager will not give focus to an override-redirect window. With noautohide="true", we should not use override-redirect (because the user / window manager cannot appropriately manage the popup window while the user is interacting with other windows).
However we can use input field of WM_HINTS to indicate that the popup should not be given focus by the window manager. This would make GTK_WINDOW_TOPLEVEL windows behave wrt focus like GTK_WINDOW_POPUP windows. That seems to make sense here. Does that sound appropriate to you, Neil? (The gtk_window_set_wmclass change is really just to avoid unncessary code. We shouldn't be using this function at all really as the application should use the same WM_CLASS name and class for all windows, but this is just a small step in that direction. )
Attachment #447058 - Flags: review?(enndeakin)
There is still an interesting issue re what happens when the window owning the panels doesn't have focus. The user is not able to indicate that the window manager should give the transient window focus, so must give the owning window focus to direct keypresses at controls in the transient windows. This is not really an issue with override-redirect windows because the keyboard will be grabbed if the windows require input. kwin (3.5 at least) by default works around this by hiding the transient utility and toolbar windows when their owner does not have focus. This happens whether or not they have the input field of WM_HINTS set. This feature can be turned off in the advanced settings. (This hiding of windows may seem a bit strange for panels with titlebar != false that may be moved far from the owning window. For these windows, maybe we should consider using regular eWindowType_toplevel windows that do receive focus.) There probably are other window managers that don't (always) hide the transient windows when their owner is inactive, so it probably is still worth doing something like attachment 445756 [details] [diff] [review] to request activation of the owning window when the user clicks on the popup. I'm still investigating whether this is the appropriate way to do that. (It shouldn't raise the window at least, merely request focus, and I haven't yet worked out the appropriate way to request focus.)
Attachment #447058 - Attachment is obsolete: true
Attachment #447058 - Flags: review?(enndeakin)
as described in comment 5, but with a missing line restored.
Attachment #447065 - Flags: review?(enndeakin)
It seems we have two options for noautohide popups: A) They can be first class citizens receiving focus like their owners. This would be different from regular popups. With focus-follows-mouse window managers, mousing over the popup would activate the popup, but deactivate its parent window. With click-to-focus window managers, the user would need to click the popup to activate it. Essentially the user would need to choose which window has focus. B) They can be inferior to their owners. These would behave similar to regular popups until the active window changes to another app (or window group leader). The user can focus the owner as they do with most windows. We can do something like attachment 445756 [details] [diff] [review], so that the user can click on the popup to activate the owner window (and also focus the element in the popup). However, (on focus-follows-mouse window managers) mousing over the popup would not activate the popup. (It is the owner that must be moused over.) My impression is that: B is best for small panels, where it is likely that the user will mouse over the owner before trying to send keypresses to the panel. A is best for titlebar != false that may be moved around. There is a grey area in between, including panels beside their owners. Perhaps its best to lean towards B here, and the user would need to click to focus.
Attachment #447065 - Attachment is patch: true
When I try just this latest patch, clicking on a textbox in a panel doesn't focus the textbox properly. (the window or panel is still inactive in some manner). When I try both patches together, I get the same result. It seems to work ok with just my patch alone. I regards to comment 8, method B sounds more like the way to go. What would, say, a popup of that type with a titlebar appearance's be when something was focused?
(In reply to comment #9) > When I try just this latest patch, clicking on a textbox in a panel doesn't > focus the textbox properly. (the window or panel is still inactive in some > manner). Thanks for testing, Neil. My understanding is that the focus manager (or something) expects the owning window (rather than the panel) to be the active window. The latest patch prevents the panel from becoming active. That is sufficient (only) for window managers that hide the panel when the owning window is inactive. With the panel absent, the user can only activate the owning window to show the panel and then may click on the textbox to focus that particular element (but it is still the owning window that is active). (For window managers that do not hide the panel, there is still the problem that the user would need to activate the owning window before clicking on the panel, and this doesn't really make sense when the panel is already visible.) > When I try both patches together, I get the same result. I assume this is because, with the latest patch, the panel window is not active so gFocusWindow would be NULL, and so SetFocus() would not get called with the SetFocus patch as it is now. > It seems to work ok with just my patch alone. Here that works immediately after clicking on the panel, but there is then strange behavior that mousing over the owning window and then back over the panel removes the textbox focus again. Mousing back over the owning window restores the textbox focus, but this is quite counter-intuitive. Subsequent clicks on the panel flash the owner's decorations inactive for a moment then back to active.
(In reply to comment #9) > I regards to comment 8, method B sounds more like the way to go. What would, > say, a popup of that type with a titlebar appearance's be when something was > focused? With kwin, the panel gets window manager decorations with a thinner titlebar. With the latest patch the decorations always look inactive. With only the SetFocus patch the panel looks active on mouse over, then becomes inactive on click (when the owner becomes active). Clicking again flashes the decorations switching activity to the panel for a moment then back to the owner. I think the issue here is that the window manager only activates at most one window and indicates activity only on that window. I think this suggests that A is a better method for titlebar panels.
There may be an option C that is more complicated but I think might give the desired behavior (for non-titlebar panels). Something like: C) The panels change the input field of WM_HINTS dynamically, but only the owner window requests activation. When the owner is active, the input field of the panel is false. Thus mousing from the owner to the panel or clicking on the panel does not change the active window. When the owner is inactive, the input field is set to true. This ensures that the panel will receive WM_TAKE_FOCUS messages from the window manager when the user indicates that they wish to activate the panel (either with click-to-focus or focus-follows-mouse). When the panel window gets a WM_TAKE_FOCUS message, instead of requesting activation of the panel window, it requests activation of the owner window. I think this should be possible within the bounds of X11/ICCCM/EWMH. I'll have to see if this flexibility is available through GDK.
Attachment #447065 - Flags: review?(enndeakin)
A is an ok option, so let's do just that. This appears to be what, for example, gimp does with its toolbox palette window.
Neil, do you mean A for titlebar panels only? For situations like the testcase here with non-titlebar panels, A could get confusing with no (titlebar) indication of the active window. What are the priorities and timeframes here? Are titlebar panels wanted first or non-titlebar panels? And which front-end features are needing these?
Depends on: 568101
(In reply to comment #14) > Are titlebar panels wanted first or non-titlebar panels? > And which front-end features are needing these? The Inspect panel wants floating panels with titlebars. (Tools -> Inspect). Various Firefox notification UI wants to use more panels but I think mostly they want non-noautohide panels which already work ok. Panels should work as follows: 1. Non-noautohide panels disappear when clicking outside of them. They do not support titlebars but can be either at the 'parent' level or 'top' level. 2. 'Parent' level noautohide panels appear just above their parent. Their position is maintained relative to the owning window if the window is moved. Titlebars don't need to be supported on these if it isn't feasible. This type is used for the Customize Toolbar dialog on Mac. 3. 'Floating' level noautohide panels appear above their parent. They may or may not have a titlebar and can be moved independently. An optional feature is to have them disappear while the application or owner window is not active. 4. 'Top' level noautohide panels appear on top of windows of all applications. They may or may not have a titlebar. The active titlebar look should apply to any panel with a titlebar while focused. The owning window may or may not have the active look (whichever is better to implement). The active titlebar should apply to the owning window for panels without titlebars. Regardless, clicking on a textbox in any type of popup focuses it such that it can be typed into. This click-to-focus-textbox behaviour is preferred over the active titlebar state being correct. I don't know enough about the focus-follows-mouse style UI to know if the behaviour should be treated differently.
(In reply to comment #15) > (In reply to comment #14) > > Various Firefox notification UI wants to use more panels but I think mostly > they want non-noautohide panels which already work ok. > 1. Non-noautohide panels disappear when clicking outside of them. They do not > support titlebars but can be either at the 'parent' level or 'top' level. Yes, these should mostly work fine, though we're getting a bit lucky that bug 568101 doesn't prevent content in iframes from being focused in non-noautohide panels. I'm not grasping why anyone would want a 'parent' level non-noautohide panel. IIUC this is so that the parent could be (partially?) hidden by another application or browser main window, but there is no way to raise the panel to make it (completely) visible. (Clicking to lower the covering window would initiate hiding of the panel.) > 2. 'Parent' level noautohide panels appear just above their parent. Their > position is maintained relative to the owning window if the window is moved. > Titlebars don't need to be supported on these if it isn't feasible. This type > is used for the Customize Toolbar dialog on Mac. I think we can do non-titlebar versions of these with only widget modifications, probably best with option C, though B is a fallback option that would require clicking the panel to focus elements. The window manager may choose whether or not to hide these when the owner is not active. Usually we should not try to override the window manager without good reason, but we may be able to provide some different hints such that the window manager is less likely to do this, if we think this is necessary.
Comment on attachment 445756 [details] [diff] [review] focus parent window on button press (In reply to comment #15) > Regardless, clicking on a textbox in any type of popup focuses it such that it > can be typed into. This click-to-focus-textbox behaviour is preferred over the > active titlebar state being correct. > > I don't know enough about the focus-follows-mouse style UI to know if the > behaviour should be treated differently. If the user clicks on textbox, mouses over another window and back to the window with the textbox, they would expect the textbox to have focus without requiring another click. Really we should be using focus-in events rather than clicks to control toplevel activation (NS_ACTIVATE). (Clicks to control element focus is fine/expected.) r- because of issues in comment 10, and I think we can do much better.
Attachment #445756 - Flags: review?(karlt) → review-
(In reply to comment #15) > The active titlebar look should apply to any panel with a titlebar while > focused. The owning window may or may not have the active look (whichever is > better to implement). The active titlebar should apply to the owning window for > panels without titlebars. I don't know any way to give the active look to the titlebars of two different windows concurrently, so I suggest A here. However, I think this will require focus manager support. If the user has clicked in a textbox in a titlebar panel and then activates the owner window, they would not expect their typing to go into the inactive panel. I think this needs the focus manager to consider the titlebar panel as its own possibly-active toplevel window, and to consider the textbox in the titlebar panel as being part of the panel window, not the owner window. Is this possible? I guess the other possibility is that the app draws the titlebar. This will lead to a titlebar that looks different from the rest of the system. My initial thoughts, if we were to do this, are that it would make more sense to use xul to set up the titlebar controls rather than asking widget to do the drawing. Client-side decorations are evolving. They may provide more options in the future.
> I'm not grasping why anyone would want a 'parent' level non-noautohide panel. > IIUC this is so that the parent could be (partially?) hidden by another The initial reason that the 'level' feature was added was because the ime popup on some platform(s) was incorrectly appearing below it. However, some popups wanted to be topmost anyway as they don't have text entry fields in them. > Really we should be using focus-in events rather than clicks to control > toplevel activation (NS_ACTIVATE). (Clicks to control element focus is > fine/expected.) Are you treating NS_ACTIVATE as fired when: 1. The window level is changed so that the window is now in front 2. The focus is shifted to that window. ? My interpretation of focus-follows-mouse is that 2 would be used. On my Ubuntu system, in this mode, the titlebar activates when I mouse over it.
(In reply to comment #19) Yes, 2, NS_ACTIVATE as fired when focus is shifted to that window, which is when the titlebar activates. (Window stacking order is independent. I don't know of a good way to get notifications of changes in stacking order.)
This applies on top of attachment 447406 [details] [diff] [review] and attachment 447681 [details] [diff] [review]. It needs a more efficient way of finding noautohide transients from their owner. (I have ~500 toplevel popup windows; I'm guessing they are from menus that are kept for the next time they might get used.)
Attachment #447065 - Attachment is obsolete: true
Making the parent window active when noautohide panels receive focus. Resolved the issues in attachment 447709 [details] [diff] [review].
Attachment #447709 - Attachment is obsolete: true
Attachment #448328 - Attachment is obsolete: true
Attachment #448458 - Flags: review?(enndeakin)
I tried this patch and the patch in bugs 568404 and 568101, but I still cannot click on a textbox in a panel. This is a very simple testcase: <button label="Panel Button noautohide" type="panel"> <panel noautohide="true"> <textbox onfocus="dump('****focus:' + event.originalTarget.localName)" onclick="dump('****click')" onblur="dump('****blur')"/> </panel> </button> When the button is pressed, the panel opens, but the main window becomes deactive. Activating the main window causes clicks to work, but they do not work until then. The click event still fires though.
OK, thanks. I'll set up metacity here to see if I can work out what is happening there. (Sounds like focus is not being automatically transferred from popup to the main window.)
Attachment #448458 - Flags: review?(enndeakin)
Hmm, when the popup gets mapped, metacity (2.24.0) is removing focus from the main window, but not even giving focus to any other window. That seems strange to me. I'll need to look at the source code to work out what metacity is trying to do.
Assignee: enndeakin → karlt
(In reply to comment #26) > Hmm, when the popup gets mapped, metacity (2.24.0) is removing focus from the > main window, but not even giving focus to any other window. That seems to be a widespread issue with metacity when transient windows are shown but don't receive focus. I've filed https://bugzilla.gnome.org/show_bug.cgi?id=621848 on this. We could probably work around this by using _NET_WM_WINDOW_TYPE_MENU instead of _NET_WM_WINDOW_TYPE_UTILITY so that place_on_top_on_map is not set to FALSE. http://git.gnome.org/browse/metacity/tree/src/core/window.c?id=b272c4ca7f3a784d011eea5c3fd348011f8558b3#n2014 But _NET_WM_WINDOW_TYPE_UTILITY seems appropriate from the description here: http://standards.freedesktop.org/wm-spec/1.4/ar01s05.html#id2568975 This is really a separate issue and doesn't need to be fixed to resolve the click to focus issue reported in comment 0.
The biggest issue with the previous patch on metacity was that metacity was ignoring (within its rights) the request to focus the parent window because the timestamp was older than the time of last user action. The place to obtain the timestamp is the WM_TAKE_FOCUS message. Filtering for that message makes all of this much simpler anyway.
Attachment #445756 - Attachment is obsolete: true
Attachment #448458 - Attachment is obsolete: true
Attachment #451822 - Flags: review?(enndeakin)
Comment on attachment 451822 [details] [diff] [review] don't let (level=parent) popups accept focus v2 This works great! The only issue is that when a panel is opened the parent window loses focus. Not a big concern though and could be investigated in a followup bug. How feasible is creating noautohide popups that stay on top?
Attachment #451822 - Flags: review?(enndeakin) → review+
(In reply to comment #29) > The only issue is that when a panel is opened the parent window loses focus. > Not a big concern though and could be investigated in a followup bug. Yes, that's the metacity issue. We could use _NET_WM_WINDOW_TYPE_MENU instead of _NET_WM_WINDOW_TYPE_UTILITY when the window manager is metacity. Should check that any fancy effects that metacity does when compositing look reasonable though. I'm guessing an animated drop-down effect would probably look OK for most panels. If a drop-down effect is always sensible for these panels then maybe we could set safely use _NET_WM_WINDOW_TYPE_MENU. (There's some history here that I can't remember OTTOMH.) > How feasible is creating noautohide popups that stay on top? Making them stay on top of all application windows is probably just gtk_window_set_keep_above(). The tricky part might be working out when it should no longer obscure windows from other apps. I'm not clear on the purpose for staying on top. Perhaps it makes more sense for popups with titlebars, but those popups I think will need the focus manager to recognize them as a separate toplevel window (comment 18).
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a6
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: