Bug 944737 - Allow only one color picker at a time. r=dholbert,jmathies
authorArnaud Bienner <arnaud.bienner@gmail.com>
Sat, 11 Jan 2014 18:46:20 +0100
changeset 163048 3f7225e6d33a5590838eb534ebf4de384184d216
parent 163047 0c63a95e2dd8157e8d7f0c3911b67dbc0d6a4a8f
child 163049 2c474a82e2b0be156ec829fd367db86436dbfe2c
push id25979
push usercbook@mozilla.com
push dateMon, 13 Jan 2014 11:46:02 +0000
treeherdermozilla-central@ea6657f1d682 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, jmathies
bugs944737
milestone29.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 944737 - Allow only one color picker at a time. r=dholbert,jmathies
widget/windows/nsColorPicker.cpp
widget/windows/nsColorPicker.h
--- a/widget/windows/nsColorPicker.cpp
+++ b/widget/windows/nsColorPicker.cpp
@@ -3,16 +3,17 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsColorPicker.h"
 
 #include <shlwapi.h>
 
+#include "mozilla/AutoRestore.h"
 #include "nsIWidget.h"
 #include "WidgetUtils.h"
 
 using namespace mozilla::widget;
 
 namespace
 {
 // Manages NS_NATIVE_TMP_WINDOW child windows. NS_NATIVE_TMP_WINDOWs are
@@ -84,40 +85,56 @@ BGRIntToRGBString(DWORD color, nsAString
 
   aResult.AssignLiteral("#");
   aResult.Append(ToHexString(r));
   aResult.Append(ToHexString(g));
   aResult.Append(ToHexString(b));
 }
 } // 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)
 {
 }
 
 NS_IMETHODIMP
 AsyncColorChooser::Run()
 {
-  CHOOSECOLOR options;
-  static COLORREF customColors[16] = {0} ;
+  static COLORREF sCustomColors[16] = {0} ;
+
+  MOZ_ASSERT(NS_IsMainThread(),
+      "Color pickers can only be opened from main thread currently");
 
-  AutoDestroyTmpWindow adtw((HWND) (mParentWidget.get() ?
-    mParentWidget->GetNativeData(NS_NATIVE_TMP_WINDOW) : nullptr));
+  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;
+
+    AutoDestroyTmpWindow adtw((HWND) (mParentWidget.get() ?
+      mParentWidget->GetNativeData(NS_NATIVE_TMP_WINDOW) : nullptr));
 
-  options.lStructSize   = sizeof(options);
-  options.hwndOwner     = adtw.get();
-  options.Flags         = CC_RGBINIT | CC_FULLOPEN;
-  options.rgbResult     = mInitialColor;
-  options.lpCustColors  = customColors;
+    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);
+    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) {
     mCallback->Done(mColor);
   }
 
   return NS_OK;
 }
@@ -131,22 +148,24 @@ nsColorPicker::nsColorPicker()
 
 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;
 }
 
 NS_IMETHODIMP
 nsColorPicker::Open(nsIColorPickerShownCallback* aCallback)
 {
   NS_ENSURE_ARG(aCallback);
   nsCOMPtr<nsIRunnable> event = new AsyncColorChooser(mInitialColor, mParentWidget, aCallback);
--- a/widget/windows/nsColorPicker.h
+++ b/widget/windows/nsColorPicker.h
@@ -16,36 +16,39 @@
 #include "nsThreadUtils.h"
 
 class nsIWidget;
 
 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;
 };
 
 class nsColorPicker :
   public nsIColorPicker
 {
 public:
   nsColorPicker();
   virtual ~nsColorPicker();
 
   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;
 };
 
 #endif // nsColorPicker_h__