Bug 944737 - Allow only one color picker at a time. r=dholbert,jmathies

widget/windows/nsColorPicker.cpp

@@ -8,6 +8,7 @@
 
 #include <shlwapi.h>
 
+#include "mozilla/AutoRestore.h"
 #include "nsIWidget.h"
 #include "WidgetUtils.h"
 
@@ -89,7 +90,9 @@ BGRIntToRGBString(DWORD color, nsAString& aResult)
 }
 } // anonymous namespace
 
-AsyncColorChooser::AsyncColorChooser(DWORD aInitialColor, nsIWidget* aParentWidget, nsIColorPickerShownCallback* aCallback)
+AsyncColorChooser::AsyncColorChooser(const nsAString& aInitialColor,
+                                     nsIWidget* aParentWidget,
+                                     nsIColorPickerShownCallback* aCallback)
   : mInitialColor(aInitialColor)
   , mParentWidget(aParentWidget)
   , mCallback(aCallback)
@@ -99,20 +102,34 @@ AsyncColorChooser::AsyncColorChooser(DWORD aInitialColor, nsIWidget* aParentWidg
 NS_IMETHODIMP
 AsyncColorChooser::Run()
 {
-  CHOOSECOLOR options;
-  static COLORREF customColors[16] = {0} ;
+  static COLORREF sCustomColors[16] = {0} ;
 
-  AutoDestroyTmpWindow adtw((HWND) (mParentWidget.get() ?
-    mParentWidget->GetNativeData(NS_NATIVE_TMP_WINDOW) : nullptr));
+  MOZ_ASSERT(NS_IsMainThread(),
+      "Color pickers can only be opened from main thread currently");
 
-  options.lStructSize   = sizeof(options);
-  options.hwndOwner     = adtw.get();
-  options.Flags         = CC_RGBINIT | CC_FULLOPEN;
-  options.rgbResult     = mInitialColor;
-  options.lpCustColors  = customColors;
+  static bool sColorPickerOpen = false;
+  // Allow only one color picker to be opened at a time, to workaround bug 944737
+  if (!sColorPickerOpen) {
+    mozilla::AutoRestore<bool> autoRestoreColorPickerOpen(sColorPickerOpen);
+    sColorPickerOpen = true;
 
-  if (ChooseColor(&options)) {
-    BGRIntToRGBString(options.rgbResult, mColor);
+    AutoDestroyTmpWindow adtw((HWND) (mParentWidget.get() ?
+      mParentWidget->GetNativeData(NS_NATIVE_TMP_WINDOW) : nullptr));
+
+    CHOOSECOLOR options;
+    options.lStructSize   = sizeof(options);
+    options.hwndOwner     = adtw.get();
+    options.Flags         = CC_RGBINIT | CC_FULLOPEN;
+    options.rgbResult     = ColorStringToRGB(mInitialColor);
+    options.lpCustColors  = sCustomColors;
+
+    if (ChooseColor(&options)) {
+      BGRIntToRGBString(options.rgbResult, mColor);
+    }
+  } else {
+    NS_WARNING("Currently, it's not possible to open more than one color "
+               "picker at a time");
+    mColor = mInitialColor;
   }
 
   if (mCallback) {
@@ -136,12 +153,14 @@ nsColorPicker::~nsColorPicker()
 NS_IMPL_ISUPPORTS1(nsColorPicker, nsIColorPicker)
 
 NS_IMETHODIMP
-nsColorPicker::Init(nsIDOMWindow* parent, const nsAString& title, const nsAString& aInitialColor)
+nsColorPicker::Init(nsIDOMWindow* parent,
+                    const nsAString& title,
+                    const nsAString& aInitialColor)
 {
   NS_PRECONDITION(parent,
       "Null parent passed to colorpicker, no color picker for you!");
   mParentWidget =  WidgetUtils::DOMWindowToWidget(parent);
-  mInitialColor = ColorStringToRGB(aInitialColor);
+  mInitialColor = aInitialColor;
   return NS_OK;
 }
 

widget/windows/nsColorPicker.h

@@ -21,11 +21,13 @@ class AsyncColorChooser :
   public nsRunnable
 {
 public:
-  AsyncColorChooser(DWORD aInitialColor, nsIWidget* aParentWidget, nsIColorPickerShownCallback* aCallback);
+  AsyncColorChooser(const nsAString& aInitialColor,
+                    nsIWidget* aParentWidget,
+                    nsIColorPickerShownCallback* aCallback);
   NS_IMETHOD Run() MOZ_OVERRIDE;
 
 private:
-  DWORD mInitialColor;
+  nsString mInitialColor;
   nsCOMPtr<nsIWidget> mParentWidget;
   nsCOMPtr<nsIColorPickerShownCallback> mCallback;
   nsString mColor;
@@ -40,11 +42,12 @@ public:
 
   NS_DECL_ISUPPORTS
 
-  NS_IMETHOD Init(nsIDOMWindow* parent, const nsAString& title, const nsAString& aInitialColor);
+  NS_IMETHOD Init(nsIDOMWindow* parent, const nsAString& title,
+                  const nsAString& aInitialColor);
   NS_IMETHOD Open(nsIColorPickerShownCallback* aCallback);
 
 protected:
-  DWORD mInitialColor;
+  nsString mInitialColor;
   nsCOMPtr<nsIWidget> mParentWidget;
 };