1496836 - <input type="color"> opened from a modal dialog is not working

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[:aceman]

When there is color picker <input type="color"> in a xul dialog opened as "modal", clicking the picker opens the platform specific color picker dialog.
On Linux, this dialog is stuck and inoperable, it receives no clicks. However, the parent (opening) dialog still receives clicks. So the picker dialog can't even be closed, but the opener can and that closes both dialogs.

This only happens if the opener dialog is "modal", without that the picker works fine.

This only happens on Linux (Gtk3.22.30), on Windows and OS X it seems to work fine.

Comment 1

[Tim Nguyen :ntim]

I can reproduce this in Firefox on a Linux VM.

STR:
- click "Set As Desktop Background…" in the context menu
- click the color input in the dialog

AR:
You can't interact with the dialog, nor can you dismiss it.

ER:
You should be able to interact with the dialog.


Karl, Daniel, do you have any idea why the color picker is frozen on Linux when initiated from "modal" opener dialogs ?

Comment 2

[Tim Nguyen :ntim]

For more context, bug 1416363 removed the custom XUL colorpicker in the chrome UI and replaced it with <input type="color">

Comment 3

[Tim Nguyen :ntim]

I'm thinking the code added in bug 917917 might be causing this.

Comment 4

[Tim Nguyen :ntim]

(In reply to Tim Nguyen :ntim (please use needinfo?) from comment #3)
> I'm thinking the code added in bug 917917 might be causing this.

Actually, it's been backed out in bug 929496.

Comment 5

[Karl Tomlinson (:karlt)]

My guess would be that the modal dialog is doing something so that its window
receives all events in the application.  The color picker dialog is not modal
and so it would not override this.

My first question would be "why is the opener modal?"
We are aiming to remove modal dialogs.  Modal is generally not useful to
the user and is often implemented with nasty hacks like nested event loops,
which cause all sorts of other problems.

If the opener dialog must be modal, then the workaround is likely to make the
color picker modal when opened from modal openers.

I wonder why other platforms don't suffer this bug?
Are their color pickers modal?

Comment 6

[Richard Marti (:Paenglab)]

On Windows it's modal, on Mac not but always in front (you can do other things in the background but the opening input field gets every colour change you do).

Comment 7

[Tim Nguyen :ntim]

(In reply to Karl Tomlinson (:karlt) from comment #5)
> My guess would be that the modal dialog is doing something so that its window
> receives all events in the application.  The color picker dialog is not modal
> and so it would not override this.
> 
> My first question would be "why is the opener modal?"
> We are aiming to remove modal dialogs.  Modal is generally not useful to
> the user and is often implemented with nasty hacks like nested event loops,
> which cause all sorts of other problems.

It’s historical, the “Set As Desktop Background” is an old feature, and there are no plans to remove or redesign it.

> If the opener dialog must be modal, then the workaround is likely to make the
> color picker modal when opened from modal openers.

Would a patch doing this be accepted? If so, could you give some pointers to detect if the parent widget is also a modal?

> I wonder why other platforms don't suffer this bug?

On Mac, it’s not modal, as for Windows, maybe it handles events differently ?

Comment 8

[Daniel Holbert [:dholbert]]

[canceling my ni; karl already provided more insight than I'd be able to, in comment 5.]

Comment 9

[Karl Tomlinson (:karlt)]

(In reply to Tim Nguyen :ntim (please use needinfo?) from comment #7)
> > If the opener dialog must be modal, then the workaround is likely to make the
> > color picker modal when opened from modal openers.
> 
> Would a patch doing this be accepted?

Yes.  I be happy for the nsIColorPicker implementation to do this, if practical, or for the nsIColorPicker interface to be modified so that the client can control modality.

> If so, could you give some pointers to
> detect if the parent widget is also a modal?

I don't know how to do that.  I suggest looking at what calls nsIWidget::SetModal() to see whether it keeps state and makes it available to clients.

Comment 10

[Tim Nguyen :ntim]

Attachment: Bug 1496836 - Set colorpicker widget as modal if parent widget is also modal on GTK. r?karlt

Comment 11

[Tim Nguyen :ntim]

This seems to fix the problem for me. Thanks!

Comment 12

[:aceman]

Comment on attachment 9015842 [details]
Bug 1496836 - Set colorpicker widget as modal if parent widget is also modal on GTK. r?karlt

Perfect, works for me too, thanks!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9a802bdfec41
Set colorpicker widget as modal if parent widget is also modal on GTK. r=karlt

Comment 14

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/9a802bdfec41