771169 - Some linux window managers steal focus on noautohide panels (Which causes some bizarre focus issues in GCLI)

Status: VERIFIED FIXED

(DevTools :: Console, defect, P1)

Description

[Joe Walker [:jwalker]]

Initially, it looks like xul:panel is ignoring noautofocus=true (particularly on linux) which make it look to GCLI as though the user has focused the web page, so the panel gets closed.

Comment 1

[Joe Walker [:jwalker]]

Neil - do you know of any reason why linux would start to ignore noautofocus=true for panels?

Comment 2

[Joe Walker [:jwalker]]

Here's some detail on what should be happening, and what I think is going wrong:

We want to manage the popup-popdown state of both panels (the output and the tooltip) so we don't want xul touching it at all.

I think linux panels might be ignoring noautofocus=true, so whenever a panel opens, it grabs the focus from the input element.

The input element sees the loss of focus as an indication that the user has clicked away from the command line, and it therefore not interested any more, so it closes the panels. So every time either panel opens, it closes straight away.

This situation is componded by errors. When there is an error, we want to help the user out in fixing the error, so we popup a tooltip panel straight away. Net result the tooltip panel opens and closes really quickly, and focus is lost from the command line, making it almost impossible to fix an errors!

It's entirely possible that my analysis is wrong, so here's a quick summary of the GCLI code:

We tell GCLI about the elements that it needs to monitor for focus events in DeveloperToolbar.jsm: 194
  this.display.focusManager.addMonitoredElement(this.outputPanel._frame);
  this.display.focusManager.addMonitoredElement(this._element);

GCLI has a focus manager that handles when to show and hide the panels. You can ask it for debug by adding a line up just above:

    tooltipClass: 'gcliterm-tooltip',
    eval: null,
    scratchpad: null,
    debug: true          // <- Add this
  });

And to get it to be extra helpful we can name the monitored elements:
  this.display.focusManager.addMonitoredElement(this.outputPanel._frame, "frame");
  this.display.focusManager.addMonitoredElement(this._element, "input");

The code for focusManager is built into gcli.jsm (line 6753) - it should take account of the various inputs and come up with a decision as to what to display (see _checkShow()) which it then forwards to DeveloperToolbar, to OutputPanel._visibilityChanged, OutputPanel._outputChanged and TooltipPanel._visibilityChanged

Comment 3

[Neil Deakin]

How would I reproduce this?

Comment 4

[Joe Walker [:jwalker]]

(In reply to Neil Deakin from comment #3)
> How would I reproduce this?

Just try to use the command line on linux. It's impossible to miss.

Comment 5

[Neil Deakin]

Since I don't know what the actual desired behaviour is, I don't know what it is I'm supposed to be missing.

When I type 'cat' in the entry field, a popup appears after I type the 'a' reading "Can't use 'ca'"

Comment 6

[Joe Walker [:jwalker]]

(In reply to Neil Deakin from comment #5)
> Since I don't know what the actual desired behaviour is, I don't know what
> it is I'm supposed to be missing.
> 
> When I type 'cat' in the entry field, a popup appears after I type the 'a'
> reading "Can't use 'ca'"

So type 'help'. Do you see any output?
I'd expect the output to open and then close immediately.

The very fact that you can read "Can't use 'ca'" seems to indicate that you don't see this though. What Linux are you on?

Comment 7

[Neil Deakin]

(In reply to Joe Walker from comment #6)

> So type 'help'. Do you see any output?
> I'd expect the output to open and then close immediately.
> 

I get no popup while typing, and a popup with various available commands appears when I press enter.

> The very fact that you can read "Can't use 'ca'" seems to indicate that you
> don't see this though. What Linux are you on?

Ubuntu 11

I might be able to get some useful info if you set 'NSPR_LOG_MODULES=WidgetFocus:5,Widget:5' in the environment before running a debug build, although it probably isn't detailed enough logging for this case.

Comment 8

[Joe Walker [:jwalker]]

(In reply to Neil Deakin from comment #7)
> (In reply to Joe Walker from comment #6)
> 
> > So type 'help'. Do you see any output?
> > I'd expect the output to open and then close immediately.
> > 
> 
> I get no popup while typing, and a popup with various available commands
> appears when I press enter.
> 
> > The very fact that you can read "Can't use 'ca'" seems to indicate that you
> > don't see this though. What Linux are you on?
> 
> Ubuntu 11
> 
> I might be able to get some useful info if you set
> 'NSPR_LOG_MODULES=WidgetFocus:5,Widget:5' in the environment before running
> a debug build, although it probably isn't detailed enough logging for this
> case.

So, I've just done a fetch/rebuild and got something different. Far less broken, but still not all right.

Type "inspect ?". You should get an error message "Syntax error in CSS Query", but the focus should be stolen from the command line.


So I ran FF with the environment you specified and got this:

968173376[7f5b388166a0]: key_release_event_cb
968173376[7f5b388166a0]: OnKeyReleaseEvent [7f5b38861380]
968173376[7f5b388166a0]: WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x8053000C: file /home/joe/Projects/mozilla/devtools/content/base/src/nsGenericElement.cpp, line 4510
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x8053000C: file /home/joe/Projects/mozilla/devtools/content/base/src/nsGenericElement.cpp, line 4510
968173376[7f5b388166a0]: WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x8053000C: file /home/joe/Projects/mozilla/devtools/content/base/src/nsGenericElement.cpp, line 4540
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x8053000C: file /home/joe/Projects/mozilla/devtools/content/base/src/nsGenericElement.cpp, line 4540
968173376[7f5b388166a0]: nsWindow [7f5b16216980]
968173376[7f5b388166a0]: 	mShell 7f5b16c02c80 7f5b16c02da0 44002fb
968173376[7f5b388166a0]: 	mContainer 7f5b10561780 7f5b16c02fe0 4400302
968173376[7f5b388166a0]: nsWindow::Show [7f5b16216980] state 1
968173376[7f5b388166a0]: 	bounds are insane or window hasn't been created yet
968173376[7f5b388166a0]: nsWindow::NativeResize [7f5b16216980] 93 866 300 89
968173376[7f5b388166a0]: size_allocate [7f5b16216980] 0 0 300 89
968173376[7f5b388166a0]: nsWindow::OnWindowStateEvent [7f5b16216980] changed 1 new_window_state 0
968173376[7f5b388166a0]: nsWindow::OnWindowStateEvent [7f5b16216980] changed 1 new_window_state 0
968173376[7f5b388166a0]: configure event [7f5b16216980] 93 866 300 89
968173376[7f5b388166a0]: GetScreenBounds 93,866 | 300x89
968173376[7f5b388166a0]: configure event [7f5b16216980] 93 866 300 89
968173376[7f5b388166a0]: GetScreenBounds 93,866 | 300x89
968173376[7f5b388166a0]: configure event [7f5b16216980] 93 866 300 89
968173376[7f5b388166a0]: GetScreenBounds 93,866 | 300x89
968173376[7f5b388166a0]: OnContainerFocusOutEvent [7f5b38861080]
968173376[7f5b388166a0]: Done with container focus out [7f5b38861080]
968173376[7f5b388166a0]: GetScreenBounds 79,152 | 1022x807
968173376[7f5b388166a0]: GetScreenBounds 79,152 | 1022x807
968173376[7f5b388166a0]: GetScreenBounds 79,152 | 1022x807
968173376[7f5b388166a0]: GetScreenBounds 79,152 | 1022x807

Comment 9

[Joe Walker [:jwalker]]

(In reply to Joe Walker from comment #8)
> ...
> Type "inspect ?". ...
> 
> So I ran FF with the environment you specified and got this:
> 
> 968173376[7f5b388166a0]: key_release_event_cb
> ...

I should add that this all came when I pressed '?'. This causes the popup (because ? isn't a valid CSS expression)

Comment 10

[Neil Deakin]

I get the popup with the syntax error message appearing, but it remains onscreen.

The log you posted shows that you are receiving an extra 'window lowering' notification that I don't get. (The line with OnContainerFocusOutEvent)

I get the same resize notifications so it may not be related to the size changes. You could try uncommenting the defines in nsFocusManager.cpp to see if we're doing anything different with focus here. If that doesn't change the log in an interesting way, it could be a difference in the platform or window manager.

Comment 11

[Joe Walker [:jwalker]]

The pop-up/pop-down behaviour is something that both Mike and I were seeing last week. For some reason it seems to be less aggressive now in that it's not removing the panel, just losing focus.

I'm not clear why that would have changed over the weekend. It feels doubly broken now because the panel should go away when the command line loses focus.

Will post on results with uncommented defines when it's rebuilt.

I'm on Ubuntu 12.04

Mike - What Linux are you on, and do you still see the pop-up/down thing?

Comment 12

[Joe Walker [:jwalker]]

With the defines commented out and NSPR_LOG_MODULES:

1734920000[7faf663166a0]: WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x8053000C: file /home/joe/Projects/mozilla/devtools/content/base/src/nsGenericElement.cpp, line 4510
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x8053000C: file /home/joe/Projects/mozilla/devtools/content/base/src/nsGenericElement.cpp, line 4510
1734920000[7faf663166a0]: WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x8053000C: file /home/joe/Projects/mozilla/devtools/content/base/src/nsGenericElement.cpp, line 4540
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x8053000C: file /home/joe/Projects/mozilla/devtools/content/base/src/nsGenericElement.cpp, line 4540
<<SetFocus>>
1734920000[7faf663166a0]: nsWindow::Show [7faf43d37f80] state 1
1734920000[7faf663166a0]: nsWindow::NativeResize [7faf43d37f80] 114 881 300 89
1734920000[7faf663166a0]: size_allocate [7faf43d37f80] 0 0 300 89
1734920000[7faf663166a0]: nsWindow::OnWindowStateEvent [7faf43d37f80] changed 1 new_window_state 0
1734920000[7faf663166a0]: nsWindow::OnWindowStateEvent [7faf43d37f80] changed 1 new_window_state 0
1734920000[7faf663166a0]: configure event [7faf43d37f80] 114 881 300 89
1734920000[7faf663166a0]: GetScreenBounds 114,881 | 300x89
1734920000[7faf663166a0]: configure event [7faf43d37f80] 114 881 300 89
1734920000[7faf663166a0]: GetScreenBounds 114,881 | 300x89
1734920000[7faf663166a0]: configure event [7faf43d37f80] 114 881 300 89
1734920000[7faf663166a0]: GetScreenBounds 114,881 | 300x89
1734920000[7faf663166a0]: OnContainerFocusOutEvent [7faf66361080]
Window 0x7faf53808400 Lowered [Currently: 0x7faf53808400 0x7faf53808400] <<[0x7faf53808400] Lowered Window: chrome://browser/content/browser.xul [0x7faf53808400] Active Window: chrome://browser/content/browser.xul>>
**Element input has been blurred
1734920000[7faf663166a0]: Done with container focus out [7faf66361080]
1734920000[7faf663166a0]: OnLeaveNotify: 7faf66361380

Comment 13

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

> Mike - What Linux are you on, and do you still see the pop-up/down thing?

I am on Fedora 16 with Gnome 3.

Comment 14

[Joe Walker [:jwalker]]

Neil, is there any other information that I can get you to help work out what is going on?

Comment 15

[Joe Walker [:jwalker]]

I think I understand why the behaviour of this bug changed between comment 2 and comment 5 onwards - We landed bug 769672 and I suspect this changed how things interact. I have half an explanation in my mind which I'll work through tomorrow.

This doesn't fix the main issue, that xul panels seem to be ignoring noautofocus=true on linux however. :(

Comment 16

[Neil Deakin]

The last log doesn't show that we're doing anything to raise or lower (focus) the panel or the main window, so the window manager seems to be lowering it on its own. The log doesn't show us doing any focusing, window level changing or doing any key focus grabs. 

If it is a window manager specific thing, it might involve fiddling with the various things set within gtk2/nsWindow::Create to figure out what specifically causes the issue. Or, you might just ask karlt if he has any insight. Since it doesn't happen for me, it's difficult for me to debug. I can ask around tommorrow to see if anyone else locally can reproduce the problem, and I can possibly try debugging from there.

Comment 17

[Joe Walker [:jwalker]]

Neil, please could you try with this patch, which I hope will land very soon:
  https://bugzilla.mozilla.org/attachment.cgi?id=641019

This undoes the change between comment 5 and 2, and makes the problem more serious. Try typing 'cat' now.

Comment 18

[Joe Walker [:jwalker]]

I've added Karlt to the CC list.

Karlt, might you have time to help us work out what's going on? The best reproduction steps for Nightly are in comment 8.

Comment 19

[Neil Deakin]

(In reply to Joe Walker from comment #17)
> This undoes the change between comment 5 and 2, and makes the problem more
> serious. Try typing 'cat' now.

Still get a popup visible.

Comment 20

[Dave Camp (:dcamp)]

I can see this on Fedora Core 15 using gnome-shell, fwiw

Comment 21

[Dave Camp (:dcamp)]

Attachment: small tweaks

The attached patch removes some stuff to make this log a little bit more clear (doubled up call to openPopup, added a printf to nsXULPopupManager::FirePopupShowingEvent).

Running this on Fedora Core 15

[dave@dave-PC dist]$ rpm -q gnome-shell
gnome-shell-3.0.2-6.fc15.x86_64

245299008[7f4d0e765150]: OnKeyReleaseEvent [7f4cf968f600]
245299008[7f4d0e765150]: nsWindow [7f4cf22f5180]
245299008[7f4d0e765150]: 	mShell 7f4cf14339b0 7f4cf3598a00 2600245
245299008[7f4d0e765150]: 	mContainer 7f4cef5a7f80 7f4cf62585c0 2600248
POPUP SHOWING EVENT
245299008[7f4d0e765150]: nsWindow::Show [7f4cf22f5180] state 1
245299008[7f4d0e765150]: nsWindow::NativeResize [7f4cf22f5180] 851 1013 300 150
245299008[7f4d0e765150]: size_allocate [7f4cf22f5180] 0 0 300 150
245299008[7f4d0e765150]: nsWindow::OnWindowStateEvent [7f4cf22f5180] changed 1 new_window_state 0
245299008[7f4d0e765150]: nsWindow::OnWindowStateEvent [7f4cf22f5180] changed 1 new_window_state 0
245299008[7f4d0e765150]: configure event [7f4cf22f5180] 851 1013 300 150
245299008[7f4d0e765150]: GetScreenBounds 851,1013 | 300x150
245299008[7f4d0e765150]: configure event [7f4cf22f5180] 851 1013 300 150
245299008[7f4d0e765150]: GetScreenBounds 851,1013 | 300x150
245299008[7f4d0e765150]: configure event [7f4cf22f5180] 851 1013 300 150
245299008[7f4d0e765150]: GetScreenBounds 851,1013 | 300x150
245299008[7f4d0e765150]: OnContainerFocusOutEvent [7f4cf968f300]
Window 0x7f4cfb208400 Lowered [Currently: 0x7f4cfb208400 0x7f4cfb208400] <<[0x7f4cfb208400] Lowered Window: chrome://browser/content/browser.xul [0x7f4cfb208400] Active Window: chrome://browser/content/browser.xul>>
**Element input has been blurred
245299008[7f4d0e765150]: Done with container focus out [7f4cf968f300]

As far as I can tell, not knowing widget code all that well, the popup isn't actually getting focus either.  We get no focus-in event on the popup.  If I alt-tab away and back to firefox, the command line gets focus again.

Comment 22

[Dave Camp (:dcamp)]

noautohide isn't being propagated down to the widget layer, because the popup frame is !mInContentShell.  Any idea what we might be doing wrong there?

(if I force noautohide in nsWindow.cpp, the focus problem doesn't happen).

Comment 23

[Dave Camp (:dcamp)]

(In reply to Dave Camp (:dcamp) from comment #22)
> noautohide isn't being propagated down to the widget layer, because the
> popup frame is !mInContentShell.  Any idea what we might be doing wrong
> there?
> 
> (if I force noautohide in nsWindow.cpp, the focus problem doesn't happen).

I misread something, ignore that.

Comment 24

[Dave Camp (:dcamp)]

After a bit of confusion because sometimes I was tweaking OutputPanel and sometimes I was tweaking Tooltip panel:

This seems to only affect the panel when noautohide=true.

Comment 25

[Joe Walker [:jwalker]]

Thanks for your help dcamp.

Comment 26

[Dave Camp (:dcamp)]

Changed the summary - please ignore comment 23, I was completely mistaken.  noautohide is applied correctly - the problem only happens when noautohide=true.

Comment 27

[Karl Tomlinson (:karlt)]

This sounds like https://bugzilla.gnome.org/show_bug.cgi?id=621848

I'm guessing Dave and Mike both have mutter on their Fedora systems.  Mutter
AIUI is derived from metacity and so may have inherited the same bug.

Joe: what window manager do you have running on Ubuntu 12?
One way to find out might be to run xlsclients and look for a window manager:
e.g. metacity or compiz.

You may like to experiment with GDK_WINDOW_TYPE_HINT_MENU at
http://hg.mozilla.org/mozilla-central/annotate/46804c31366b/widget/gtk2/nsWindow.cpp#l3546
to confirm this as the cause (bug 526941 comment 27).

Comment 28

[Saurabh Anand [:sawrubh]]

I tried using the command line, by pressing "Shift + F2", then I typed restart in it, then I wanted to press backspace and erase it.I deleted a character but after that it's loosing focus.Every time I try to delete the text or select some text in that remaining command.I closed the command line and then reopened it (note : I didn't restart the browser), 	but the problem and that remaining text was still there and I cannot remove/delete it.I think there should be a graceful alert or something, currently there is a momentary notif which appears and then disappears, and I read fleetingly "Can't use retart", "retart" is the remaining word after I removed the "s".

My system info :

OS : Ubuntu 12.04 , Gnome Shell 3.2
Firefox version : 16.a01 (2012-7-15)

Please tell me if any more information is required.

Thanks

Comment 29

[Joe Walker [:jwalker]]

I'm using stock Ubuntu 12/Unity -> window manager = metacity

I think we're going to need to fix this both on Nightly and Aurora.

Comment 30

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Switched from utility to menu

karlt: You are correct, changing from GDK_WINDOW_TYPE_HINT_UTILITY to GDK_WINDOW_TYPE_HINT_MENU does indeed resolve the problem (see attachment).

I don't know where we want to go from here as it looks like the Utility type was broken when modal dialog support was added (having a dialog and it's parent both focused at the same time seemed odd). At the same time I don't see any negative side effects to using this workaround until the metacity bug is fixed as GCLI is unusable on Linux without it.

Anyhow, what do you think?

Comment 31

[Neil Deakin]

Doing that for all panels is certainly going to cause problems. If you want to use a workaround for specific panels, I'd suggest adding a separate flag to the initdata that can be assigned from an attribute within nsMenuPopupFrame.

Comment 32

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

A c++ coder would be better doing this ... any offers?

Comment 33

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Added nostealfocus attribute to use GDK_WINDOW_TYPE_HINT_MENU

I have created a new attribute to temporarily work around this issue until https://bugzilla.gnome.org/show_bug.cgi?id=621848 is fixed.

The attribute is nostealfocus and it can be set to true on autofocus panels in order for them to be treated as tear-offs (GDK_WINDOW_TYPE_HINT_MENU) in order to prevent them from stealing focus.

Comment 34

[Dão Gottwald [:dao]]

Can you use an attribute name that makes it clearer that it's a temporary workaround and usually shouldn't be set on panels, unlike other panel attributes?

Maybe it even makes sense to re-use the noautohide attribute as a hint?

Comment 35

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Added metacityfocusstealingworkaround attribute to use GDK_WINDOW_TYPE_HINT_MENU

(In reply to Dão Gottwald [:dao] from comment #34)
> Can you use an attribute name that makes it clearer that it's a temporary
> workaround and usually shouldn't be set on panels, unlike other panel
> attributes?
> 

I have renamed it to metacityfocusstealingworkaround as discussed on IRC.

> Maybe it even makes sense to re-use the noautohide attribute as a hint?

Well, this window type has the problem:
GDK_WINDOW_TYPE_HINT_UTILITY - Utility windows which are not detached toolbars or dialogs

And this doesn't:
GDK_WINDOW_TYPE_HINT_MENU - Window used to implement a menu; GTK+ uses this hint only for torn-off menus, see GtkTearoffMenuItem.

I doubt that we would want to use them for all noautohide panels, in fact, I doubt that we would want to use it for all noautofocus panels either. Karl & Neil are probably the best ones to advise us on this one.

Comment 36

[Dão Gottwald [:dao]]

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #35)
> I have renamed it to metacityfocusstealingworkaround as discussed on IRC.

here's the reasoning:

> mikeratcliffe: So something like _nostealfocus?
> dao: as a workaround it doesn't need to be pretty or brief.
> dao: even gnomefocusstealingworkaround would work for me :)
> dao: the underscore prefix seems like a reasonable idea as well
> mikeratcliffe: I will use your name (well, metacityfocusstealingworkaround),
> mikeratcliffe: it makes it clear that this is a workaround.

Comment 37

[Karl Tomlinson (:karlt)]

Given the window is hidden when focus is lost, is there a need for the noautohide attribute?

Is there such a thing as a "tooltip" panel that could be used here?

Comment 38

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

(In reply to Karl Tomlinson (:karlt) from comment #37)
> Given the window is hidden when focus is lost, is there a need for the
> noautohide attribute?
> 

The panel is not hidden when the focus is lost ... the focus should never be given to the panel in the first place.

The user types in an input box and the panel appears with tips and suggestions. The focus should not be moved from the input box as the user should be able to type uninterrupted.

> Is there such a thing as a "tooltip" panel that could be used here?

I can try using a tooltip in the morning (about 8 hours from now).

Comment 39

[Karl Tomlinson (:karlt)]

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #38)
> The panel is not hidden when the focus is lost ... the focus should never be
> given to the panel in the first place.

I meant when the main window loses focus.  None of these popups/panels really get focus.

> The user types in an input box and the panel appears with tips and
> suggestions. The focus should not be moved from the input box as the user
> should be able to type uninterrupted.

Yes, that's how it is meant to work.

> > Is there such a thing as a "tooltip" panel that could be used here?
> 
> I can try using a tooltip in the morning (about 8 hours from now).

I assume the main reason for using noautohide would be so that the pointer does not get grabbed/captured.  tooltips would also provide that feature and it sounds like the window_type would be a reasonable match for the use case.

Comment 40

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Change panels to tooltips

I can't believe that I hadn't tried using tooltips, it works fine this way without any ugly hacks.

joe_walker: if this works for you we should land it in fx-team and aurora.

Comment 41

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

http://tbpl.mozilla.org/?tree=Try&rev=3f60632015ac

Comment 42

[Neil Deakin]

Note that tooltips can hide themselves automatically as the mouse moves so it won't behave exactly as you might intend. If you're OK with that you might be able to remove any code you have to hide the panel manually.

The noautohide and noautofocus attributes aren't used for tooltips so they could be removed as well.

Comment 43

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 643323 [details] [diff] [review]
Change panels to tooltips

You are right ... this doesn't work for us, in effect autohide doesn't work as expected with these panels. I will see if I can work around this.

Comment 44

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Patch

So now we have tooltips instead of normal panels and we prevent them from autohiding without our permission.

Works perfectly for me on Linux.

Try:
http://tbpl.mozilla.org/?tree=Try&rev=4a351ee13c8b

Comment 45

[Joe Walker [:jwalker]]

Sorry I've not reviewed this before now, and I'm about to go on holiday for a week...
Will have a look when I get back.

Comment 46

[Rob Campbell [:rc] (:robcee)]

I'm not terribly keen on the idea of switching panels to tooltips for these things. Tooltips are supposed to be used for small, informational feedback that is transient. I think that the output panels for the GCLI should be a little more persistent than that, but I defer to Joe here.

Comment 47

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Fixed leaks

This issue makes it impossible to use GCLI for a large number of Linux users. It should be landed in fx-team and aurora as soon as possible.

Robcee doesn't like the idea of using a tooltip because, as he rightly points out, these really should be panels instead of tooltips ... they really should be something persistent.

We believe it probably makes more sense to use the other patch (https://bug771169.bugzilla.mozilla.org/attachment.cgi?id=642927) because this is a workaround for a metacity bug (https://bugzilla.gnome.org/show_bug.cgi?id=621848).

Neil, what do you think? If you are happy to use attachment 642927 [details] [diff] [review] can you review it, it is very simple?

Comment 48

[Karl Tomlinson (:karlt)]

If you end up, using attachment 642927 [details] [diff] [review], can you set widgetData.mPopupHint instead of adding another member to nsWidgetInitData.

If you are not expecting user input on the window, then "tooltip" sounds fine to me as it is just showing information, but I guess it comes down to where you want to put the workaround.

Comment 49

[Neil Deakin]

Comment on attachment 644341 [details] [diff] [review]
Fixed leaks

Seems ok. I don't think there is an ideal solution here since there's an underlying platform bug.

Comment 50

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 644341 [details] [diff] [review]
Fixed leaks

So it seems that they would prefer us to use tooltips, at least for the moment.

We should land this on fx-team and aurora as GCLI is totally unusable without it on many Linux boxes.

Comment 51

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 644341 [details] [diff] [review]
Fixed leaks

After discussing this with robcee we have decided that we will use the c++ patch with karlt's changes.

Comment 52

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: New c++ patch with karlt's changes

Comment 53

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 647121 [details] [diff] [review]
New c++ patch with karlt's changes

+
+  // Bug 771169: Due to focus issues on noautohide panels we have temporarily
+  // added a metacityfocusstealingworkaround attribute. This attribute allows us
+  // to use GDK_WINDOW_TYPE_HINT_MENU instead of GDK_WINDOW_TYPE_HINT_UTILITY to
+  // avoid these focus issues.

Your assertion that you've temporarily added an attribute is humorous. You should maybe add a TODO prefix to this comment if you plan to address it in the future.

Otherwise, I'd remove the commentary about it being a temporary fix.

+  //
+  // This change should be reverted when
+  // https://bugzilla.gnome.org/show_bug.cgi?id=621848 is fixed.
+  this._panel.setAttribute("metacityfocusstealingworkaround", "true");

Does this really need a special, ugly-named attribute for this? Why not just do this by default? IF you really need an attribute name, I'd suggest "ignorefocus" or "nofocus" etc.

+  // Bug 771169: Due to focus issues on noautohide panels we have temporarily
+  // added a metacityfocusstealingworkaround attribute. This attribute allows us
+  // to use GDK_WINDOW_TYPE_HINT_MENU instead of GDK_WINDOW_TYPE_HINT_UTILITY to
+  // avoid these focus issues.
+  //
+  // This change should be reverted when
+  // https://bugzilla.gnome.org/show_bug.cgi?id=621848 is fixed.
+  this._panel.setAttribute("metacityfocusstealingworkaround", "true");

ditto.

also, if we need the attribute I think we should rename it to "metastacy".

I'm ok with the widget changes, but someone from the widget module should review.

Comment 54

[Neil Deakin]

Can you explain why the tooltip workaround isn't satisfactory? Especially for the popup called 'TooltipPanel'?

Comment 55

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 647121 [details] [diff] [review]
New c++ patch with karlt's changes

After discussing this in our daily meeting we have decided to use the JS patch with updated comments.

Comment 56

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Metastacy Fix

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #55)
> After discussing this in our daily meeting we have decided to use the JS
> patch with updated comments.

We being mikeratcliffe, dcamp and robcee.

So, back to tooltips with better comments.

Comment 57

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

... and jwalker

Comment 58

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 647999 [details] [diff] [review]
Metastacy Fix

     <html:iframe xmlns:html="http://www.w3.org/1999/xhtml"
                  id="gcli-output-frame"
                  src="chrome://browser/content/devtools/gclioutput.xhtml"
                  flex="1"/>

unrelated nit: is html:iframe needed? just plain' ol' iframe should work there. 

+  // TODO: Switch back from tooltip to panel when metacity focus issue is fixed:
+  // https://bugzilla.gnome.org/show_bug.cgi?id=621848

Should file a bug to revert this to panel after that blocking bug is fixed and reference it in these TODOs.

Comment 59

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Metastacy Fix

(In reply to Rob Campbell [:rc] (:robcee) from comment #58)
> Comment on attachment 647999 [details] [diff] [review]
> Metastacy Fix
> 
>      <html:iframe xmlns:html="http://www.w3.org/1999/xhtml"
>                   id="gcli-output-frame"
>                   src="chrome://browser/content/devtools/gclioutput.xhtml"
>                   flex="1"/>
> 
> unrelated nit: is html:iframe needed? just plain' ol' iframe should work
> there. 
> 

Agreed but a plain' ol' iframe doesn't work, at least if we use one it isn't visible.

> +  // TODO: Switch back from tooltip to panel when metacity focus issue is
> fixed:
> +  // https://bugzilla.gnome.org/show_bug.cgi?id=621848
> 
> Should file a bug to revert this to panel after that blocking bug is fixed
> and reference it in these TODOs.

Done

Comment 60

[Victor Porof [:vporof][:vp]]

https://hg.mozilla.org/integration/fx-team/rev/0c3dd2229961

Comment 61

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/0c3dd2229961

Comment 62

[Alfredos-Panagiotis Damkalis [:fredy]]

Verified at latest nightly (17.0a1 (2012-08-10)) (Mozilla/5.0 (X11; Linux x86_64; rv:17.0)

Thanks for this fix!