917917 - Clicking <input type="color"> N times will spawn N distinct color picker dialogs.

Status: VERIFIED FIXED

(Core :: Widget: Gtk, defect)

Description

[Daniel Holbert [:dholbert]]

With the patch from bug 875275 applied, on Linux: if I click an <input type="color"> widget 10 times, I get 10 distinct color picker dialogs. (And I can continue clicking & spawning more).

I think we should make this dialog modal, like it is for <input type="file">; otherwise, users will get confused if they e.g. double-click the button (lots of people double-click buttons), and then wonder why they have to pick a color twice.  It's also a nuisance to have to dismiss N different copies of the same dialog.

(Alternately: we could make the dialog non-modal, but only allow one copy of it to be up at a time (per tab?), or something like that.)

(I'm assuming this non-modal aspect is GTK-specific, based on bug 875275 comment 45 and bug 875753 comment 5, so I'm filing this as Widget:GTK)

Comment 1

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #0)
> (Alternately: we could make the dialog non-modal, but only allow one copy of
> it to be up at a time (per tab?), or something like that.)

This is what Gimp basically does (and it focuses the existing dialog, if you click the color-picker button again).  Arnaud also suggested this as one possible alternative in bug 875275 comment 45.

I'm not a UI designer, so I don't want to express too strong of an opinion over whether that vs. modal is better. My main concern here is about allowing a single color-picker button to spawn zillions of simultaneous dialogs that control its color :) so any solution that avoids that will make me happy.

Comment 2

[Mounir Lamouri (:mounir)]

I agree that clicking N times on the same <input type='color'> should not open N colour pickers. Whether clicking on N <input type='color'> should open N colour pickers or only one is another question I guess - I don't have strong opinion on that, we do open only one colour picker on MacOS X but it is because of a system limitation.

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to Mounir Lamouri (:mounir) from comment #2)
> Whether clicking on N <input type='color'> should
> open N colour pickers or only one is another question I guess

That's a good question.

For comparison: in Gimp on Ubuntu Linux, only one dialog is ever up at a time.  There are two colors you can pick (foreground & background); if you click one (spawning a color-picker) and then click the other, it looks like the existing color-picker dialog just gets repurposed.  (Though it's a bit more complicated, because you can switch back and forth between the colors, and then save all of the changes you made by hitting "OK", or clear all the changes that you made while the color-picker was up by hitting "Cancel").

Comment 4

[Arnaud Bienner]

Do you think this issue is a show stopper to activate input color for Linux?
If so, I suggest to make the color picker window modal (easy change: one line), waiting for a more elaborated/smarter fix.
As I said in bug 875753 comment 5, I prefer windows not being modal when it doesn't sound necessary but, on the other hand, file picker window is modal on Linux/Gtk, so modifying the current behavior for color picker will not be inconsistent with other pickers.

In a second step, we might want to have something better: I tested how it works on Chrome and it is similar to what you described for GIMP: the picker isn't modal and clicking another picker close the first one and reopen and new one (so we only have one picker opened at a time).
For me, this sounds to be an ideal behavior.

Comment 5

[Daniel Holbert [:dholbert]]

(In reply to Arnaud Bienner from comment #4)
> Do you think this issue is a show stopper to activate input color for Linux?

It's probably not a show stopper for turning it on in nightly builds, but it probably should block us shipping that pref-toggle to release users.

> If so, I suggest to make the color picker window modal (easy change: one
> line), waiting for a more elaborated/smarter fix.

That sounds good to me. While it may not be perfect, I think it's better than the current behavior, and (as you note) it's at least consistent with another widget's picker-dialog behavior.

Comment 6

[Arnaud Bienner]

Attachment: Make the color picker modal

Here is a patch to make the color picker modal (until we have something better).
@Karl: I've added you to the review, as you reviewed my patch in bug 875753.

Comment 7

[Arnaud Bienner]

Comment on attachment 819508 [details] [diff] [review]
Make the color picker modal

Actually, I think one review is enough, as the patch is very small and Daniel agrees in comment 5 about having the picker being modal.

Comment 8

[Arnaud Bienner]

Attachment: Make the color picker modal (2nd)

Updated version of the patch, with a proper commit message.
r+ given by karlt on previous patch (attachment 819508 [details] [diff] [review]).

Comment 9

[Daniel Holbert [:dholbert]]

Landed:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/230b8bcc394d

Regarding [leave open] -- I think we should close this, since the bug as-described (multiple dialogs spawned) will be fixed once this is merged to central.

We can file a separate bug on doing something smarter & non-modal (like what you mentioned at the beginning of comment 4). (Bugs are cheap, and it's better to err on the side of limiting their scope.)

(Would you mind filing that followup? Setting needinfo=you so we don't forget, & clearing [leave open].)

Comment 10

[Arnaud Bienner]

I just didn't want to loose the history (as we started conversation about this here) and I would have prefer to agree on the "ideal solution" before opening any new bug.
But if we're OK about having the "non modal picker, but only once opened at a time" behavior, indeed we can close this bug and open a new one.

I would like to hear Karlt's point of view about this (what he thinks can be the ideal behavior).

Comment 11

[Daniel Holbert [:dholbert]]

(In reply to Arnaud Bienner from comment #10)
> I just didn't want to loose the history (as we started conversation about
> this here) 

Gotcha. (We can preserve that via bug dependencies -- new bug should depend on this one)

> and I would have prefer to agree on the "ideal solution" before
> opening any new bug.

OK. In the interests of that: here are the options I've seen proposed here:

 (1) Color picker dialogs are modal (the current behavior w/ patch landed in comment 9)

 (2) Allow at most 1 dialog per Firefox window, but it doesn't block you from interacting with Firefox, aside from preventing subsequent instances of that dialog. Sort of like how Tools|Options behaves. (Possible extension: if you click on a <input type="color"> button while a dialog is already up, maybe the existing dialog changes its allegiance.)

 (3) Allow at most 1 dialog per color-input button. (so, multiple dialogs can be displayed simultaneously if you have multiple buttons)

I'm not a UI guy, so I can't really comment authoritatively on which of those we should go with, but I'll just reemphasize that (1) is consistent with our <input type="file"> behavior and is hence not-too-unreasonable of a behavior.

Comment 12

[Karl Tomlinson (:karlt)]

The relevant discussion in this bug can be summarized in the new bug, and I would usually suggest filing the new bug before there is more to summarize here.

The disadvantage of (1) with the file picker is that text from the transient-for (parent/owner) window can't be scrolled or selected, etc, for use in picking a filename to save for example.  I don't know how relevant that is to color pickers.

Color and file pickers are quite different from application preferences.  Preferences apply to the whole application and so need not be associated with any particular window.

If there are two color inputs in a page, then perhaps it might be useful to have two color pickers open to compare input?  However, this is only useful if the two color pickers can be indentified and associated with their inputs.

If it is not clear which color picker is associated with which input, then I recommend that only one should be open at a time.  If the user clicks on an input, then they will expect a dialog for that input, so if there is an existing dialog for another input, then that should be replaced with a dialog for the new input.  There are finer details to determine such as whether the existing dialog is reused, but I think cancelling the existing dialog and reopening would be fine.
If there is an existing dialog for that input, then it could be raised and focused, but cancelling and reopening would also be ok.

As far as different inputs in different windows, I guess there is at least some association between the dialog and the window, so perhaps we could keep more than one open, but I suspect the association would not always be clear, and I doubt it is worth the effort of handling this situation differently.

Comment 13

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

https://hg.mozilla.org/mozilla-central/rev/230b8bcc394d

Comment 14

[Daniel Holbert [:dholbert]]

Verified fixed in my local debug build (with no patches applied), with
 data:text/html,<input type="color">
and with pref dom.forms.color set to true.