1390794 - Date picker closed immediately after it's opened

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, enhancement, P1)

Description

[Jessica Jong [:jessica] (inactive)]

This only happens on some platforms, like Ubuntu + Gnome and Linux Mint. It's not reproducible on Ubuntu Unity, MacOS nor Windows.

Looking from the upper level, I see that the input box is blurred (and also the whole Firefox window is blurred), so we close the picker.

Digging a little bit deeper, I found that when we set "noautohide=true", we call gtk_window_new() with type GTK_WINDOW_TOPLEVEL, which is managed by the window manager, instead of GTK_WINDOW_POPUP [1]. Changing to use GTK_WINDOW_POPUP always solves the problem, but I'm not familiar with gtk widget code to change this.

Maybe we should move this to "Widget: Gtk" component?

[1] http://searchfox.org/mozilla-central/rev/13148faaa91a1c823a7d68563d9995480e714979/widget/gtk/nsWindow.cpp#3637-3640

Comment 1

[Jessica Jong [:jessica] (inactive)]

HI Karl,

For date picker panel, we set "noautohide=true" because we don't want the picker to close (and open again) when clicking on different fields in the input box (e.g., click year and then click month). But setting "noautohide=true", widget's code uses GTK_WINDOW_TOPLEVEL instead of GTK_WINDOW_POPUP, and with GTK_WINDOW_TOPLEVEL, the original parent window blurs when the picker is shown. I tried to use gtk_widget_set_can_focus() to FALSE, but the original parent window still blurs. Do you think we can just use GTK_WINDOW_POPUP, even with "noautohide=true"?

Thanks.

Comment 2

[Karl Tomlinson (:karlt)]

I wonder whether the problems with GNOME are the same as
https://bugzilla.gnome.org/show_bug.cgi?id=621848

I doubt GNOME is using metacity, but there may be some copied code.

If so, see also
https://bug635044.bugzilla.mozilla.org/show_bug.cgi?id=526941#c26
https://bug635044.bugzilla.mozilla.org/show_bug.cgi?id=526941#c29

Comment 3

[Karl Tomlinson (:karlt)]

The problem with using GTK_WINDOW_POPUP is that AIUI noautohide=true was
designed for windows that would continue to be shown after the app no longer
has the active window.  For such panels GTK_WINDOW_POPUP would not be
appropriate because the window would remain above other windows of other
applications.

Is it necessary to close the panel when the owning window is no longer active?

Someone may temporarily make an another app active to check a date, and would
need to reopen the date picker.

I understand closing the panel when focus moves to a different element within
the window.
I don't know whether it is possible to tell whether the element is the focused
element when the window is not active.

Is it a design decision not to show the panel when the element receives focus?

Comment 4

[Jessica Jong [:jessica] (inactive)]

(In reply to Karl Tomlinson (:karlt) from comment #2)
> I wonder whether the problems with GNOME are the same as
> https://bugzilla.gnome.org/show_bug.cgi?id=621848
> 
> I doubt GNOME is using metacity, but there may be some copied code.
> 
> If so, see also
> https://bug635044.bugzilla.mozilla.org/show_bug.cgi?id=526941#c26
> https://bug635044.bugzilla.mozilla.org/show_bug.cgi?id=526941#c29

Yes, that is the exact problem, the original (parent) window gets blurred, not giving focus to any other window. It semms that the issue exists event after all these years... :(

(In reply to Karl Tomlinson (:karlt) from comment #3)
> The problem with using GTK_WINDOW_POPUP is that AIUI noautohide=true was
> designed for windows that would continue to be shown after the app no longer
> has the active window.  For such panels GTK_WINDOW_POPUP would not be
> appropriate because the window would remain above other windows of other
> applications.

I understand now.

> 
> Is it necessary to close the panel when the owning window is no longer
> active?

Yes, that is how a regular panel (with noautohide=false) would work. We'd like to have the same behavior, except not closing the panel when moving inside the anchored element. The picker is also tab specific, so moving to another tab should cause the panel to close as well.
Also note that we set the panel with noautofocus=true, so we expect the focus to remain on the input box even when the panel is shown.

> 
> Someone may temporarily make an another app active to check a date, and would
> need to reopen the date picker.
> 
> I understand closing the panel when focus moves to a different element within
> the window.
> I don't know whether it is possible to tell whether the element is the
> focused
> element when the window is not active.

We'd prefer to keep the current behavior, as stated above. Do you have anything in mind that can help us fix this?

> 
> Is it a design decision not to show the panel when the element receives
> focus?

Now we only show the panel on click. For accesibilty, there will be a combination of keys (Atl + Down) to open the panel, but that is not supported yet, it's in our backlog.

Comment 5

[Jessica Jong [:jessica] (inactive)]

Reading through Bug 771169, which faced the same issue as this one, it seems that we have two options here:

1) Use XUL tooltip instead of XUL panel
2) Have a special popup hint to make gtk code use GDK_WINDOW_TYPE_HINT_MENU instead of GDK_WINDOW_TYPE_HINT_UTILITY

Option 1 is somewhat risky, as we have already tunned our UI for picker, and we have limited time (this is planned to ship in v5) to find out what could go wrong. Do you think we can go for option 2?

Comment 6

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Option 1 sounds like a big refactoring on front-end UI piece. 
I hope we don't need to go that direction.

Comment 7

[Karl Tomlinson (:karlt)]

(In reply to Jessica Jong [:jessica] from comment #4)
> Yes, that is how a regular panel (with noautohide=false) would work. We'd
> like to have the same behavior, except not closing the panel when moving
> inside the anchored element.

That makes it sound like the ideal solution would be to have the
nsIRollupListener decide when to roll up a regular panel.

noautohide panels will behave somewhat differently to regular panels.
Consider mousing out of the active window, for example.  Mousing over another
window (accidentally or intentionally) may make the other window active, which
would close this noautohide panel, but the active window would not change with
a regular panel.  I don't know which behavior you prefer, but there may be
other differences.

Does the search box dropdown have the behavior that you'd like?
See bug 1116865.

(In reply to Jessica Jong [:jessica] from comment #5)
> Reading through Bug 771169, which faced the same issue as this one, it seems
> that we have two options here:
> 
> 1) Use XUL tooltip instead of XUL panel

I'd be a bit cautious about using a tooltip, because I haven't heard of
anything using a tooltip to receive mouse input.

> 2) Have a special popup hint to make gtk code use GDK_WINDOW_TYPE_HINT_MENU
> instead of GDK_WINDOW_TYPE_HINT_UTILITY

Either having a way to set nsWidetInitData::mPopupHint to ePopupTypeMenu, or
detecting broken window managers and always using _MENU for them on
noautohide panels are both options here, if there are good reasons to prefer
noautohide behavior.

Comment 8

[Jessica Jong [:jessica] (inactive)]

(In reply to Karl Tomlinson (:karlt) from comment #7)

> That makes it sound like the ideal solution would be to have the
> nsIRollupListener decide when to roll up a regular panel.
> 
> noautohide panels will behave somewhat differently to regular panels.
> Consider mousing out of the active window, for example.  Mousing over another
> window (accidentally or intentionally) may make the other window active,
> which
> would close this noautohide panel, but the active window would not change
> with
> a regular panel.  I don't know which behavior you prefer, but there may be
> other differences.
> 
> Does the search box dropdown have the behavior that you'd like?
> See bug 1116865.

The "norolluponanchor" attribute does seem to do the behavior we'd like to have. Thanks for the advice!
I tried it on my Linux machine and it works great, but it doesn't work well on macOS: the part to detect if the mouse click is inside the anchored element always fail [1]. Looks like the cocoa widget code is not passing the `pos` argument in device pixels. I'll file a separate bug for this.

> 
> (In reply to Jessica Jong [:jessica] from comment #5)
> > Reading through Bug 771169, which faced the same issue as this one, it seems
> > that we have two options here:
> > 
> > 1) Use XUL tooltip instead of XUL panel
> 
> I'd be a bit cautious about using a tooltip, because I haven't heard of
> anything using a tooltip to receive mouse input.
> 
> > 2) Have a special popup hint to make gtk code use GDK_WINDOW_TYPE_HINT_MENU
> > instead of GDK_WINDOW_TYPE_HINT_UTILITY
> 
> Either having a way to set nsWidetInitData::mPopupHint to ePopupTypeMenu, or
> detecting broken window managers and always using _MENU for them on
> noautohide panels are both options here, if there are good reasons to prefer
> noautohide behavior.

I see. This will be our last resort if "norolluponanchor" does not work. But how do we detect "broken window managers" in gtk widget code?


[1] http://searchfox.org/mozilla-central/rev/48ea452803907f2575d81021e8678634e8067fc2/layout/xul/nsXULPopupManager.cpp#303

Comment 9

[Jessica Jong [:jessica] (inactive)]

Attachment: Use 'norolluponanchor' instead of 'noautohide'.

Tested on macOS (on top of bug 1392549), Ubuntu and Windows.

Comment 10

[Jessica Jong [:jessica] (inactive)]

Thanks Karl for your suggestion.

Changing back to Layout: Form Controls, since we are not changing the widget code but fixing it with the 'norolluponanchor' attribute.

Comment 11

[Jessica Jong [:jessica] (inactive)]

Comment on attachment 8900125 [details] [diff] [review]
Use 'norolluponanchor' instead of 'noautohide'.

Hi Mike, could you help review this? Thanks.

Comment 12

[Ethan Jaszewski]

Worth noting I filed the same bug a few days ago might want to link it or close it. Present on Ubuntu 16.04 and Linux Mint 18.2, both with Cinnamon 3.4.
bug 1389725

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8900125 [details] [diff] [review]
Use 'norolluponanchor' instead of 'noautohide'.

Review of attachment 8900125 [details] [diff] [review]:
-----------------------------------------------------------------

This is a great solution, good work!

Comment 14

[Jessica Jong [:jessica] (inactive)]

(In reply to Ethan Jaszewski from comment #12)
> Worth noting I filed the same bug a few days ago might want to link it or
> close it. Present on Ubuntu 16.04 and Linux Mint 18.2, both with Cinnamon
> 3.4.
> bug 1389725

Thanks for the input. We can verify if bug 1389725 is fixed after the patch in this bug lands. If so, we can close bug 1389725 as a duplicate of this one.

Comment 15

[Jessica Jong [:jessica] (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=478d3e30f1d59c4cbf45ee90e7b1253f1199d778&group_state=expanded

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c9acfd655f8
Use 'norolluponanchor' to avoid closing the picker when the anchored input box is clicked. r=mconley

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8c9acfd655f8

Comment 18

[Ethan Jaszewski]

This fix also resolved bug 1389725. It can be closed as a duplicate now.