Bug 975468 - Allow only one NSColorPanelWrapper to be opened at a time, and retarget the existing one if user clicks on another input type color. r=areinald
☠☠ backed out by ecedfea04ffc ☠ ☠
authorArnaud Bienner <arnaud.bienner@gmail.com>
Thu, 13 Mar 2014 16:04:57 +0100
changeset 191637 16d6b2e332e09ebba1fa623e300b28ceddd9c858
parent 191636 68b0f7e79ceb9fab9ebfa37d219697fdc5cb48d4
child 191638 f0cd5b6b1ec2d5496f22ff93df63d3294dd84aca
push id474
push userasasaki@mozilla.com
push dateMon, 02 Jun 2014 21:01:02 +0000
treeherdermozilla-release@967f4cf1b31c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersareinald
bugs975468
milestone30.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 975468 - Allow only one NSColorPanelWrapper to be opened at a time, and retarget the existing one if user clicks on another input type color. r=areinald
widget/cocoa/nsColorPicker.h
widget/cocoa/nsColorPicker.mm
--- a/widget/cocoa/nsColorPicker.h
+++ b/widget/cocoa/nsColorPicker.h
@@ -22,21 +22,27 @@ public:
   NS_DECL_ISUPPORTS
 
   NS_IMETHOD Init(nsIDOMWindow* aParent, const nsAString& aTitle,
                   const nsAString& aInitialColor);
   NS_IMETHOD Open(nsIColorPickerShownCallback* aCallback);
 
   // For NSColorPanelWrapper.
   void Update(NSColor* aColor);
+  // Call this method if you are done with this input, but the color picker needs to
+  // stay open as it will be associated to another input
+  void DoneWithRetarget();
+  // Same as DoneWithRetarget + clean the static instance of sColorPanelWrapper,
+  // as it is not needed anymore for now
   void Done();
 
 private:
   static NSColor* GetNSColorFromHexString(const nsAString& aColor);
   static void GetHexStringFromNSColor(NSColor* aColor, nsAString& aResult);
 
+  static NSColorPanelWrapper* sColorPanelWrapper;
+
   nsString             mTitle;
   nsString             mColor;
-  NSColorPanelWrapper* mColorPanel;
   nsCOMPtr<nsIColorPickerShownCallback> mCallback;
 };
 
 #endif // nsColorPicker_h_
--- a/widget/cocoa/nsColorPicker.mm
+++ b/widget/cocoa/nsColorPicker.mm
@@ -32,16 +32,17 @@ HexStrToInt(NSString* str)
 
 @interface NSColorPanelWrapper : NSObject <NSWindowDelegate>
 {
   NSColorPanel*  mColorPanel;
   nsColorPicker* mColorPicker;
 }
 - (id)initWithPicker:(nsColorPicker*)aPicker;
 - (void)open:(NSColor*)aInitialColor title:(NSString*)aTitle;
+- (void)retarget:(nsColorPicker*)aPicker;
 - (void)colorChanged:(NSColorPanel*)aPanel;
 @end
 
 @implementation NSColorPanelWrapper
 - (id)initWithPicker:(nsColorPicker*)aPicker
 {
   mColorPicker = aPicker;
   mColorPanel = [NSColorPanel sharedColorPanel];
@@ -65,16 +66,22 @@ HexStrToInt(NSString* str)
   mColorPicker->Update([mColorPanel color]);
 }
 
 - (void)windowWillClose:(NSNotification*)aNotification
 {
   mColorPicker->Done();
 }
 
+- (void)retarget:(nsColorPicker*)aPicker
+{
+  mColorPicker->DoneWithRetarget();
+  mColorPicker = aPicker;
+}
+
 - (void)dealloc
 {
   if ([mColorPanel delegate] == self) {
     [mColorPanel setTarget:nil];
     [mColorPanel setAction:nil];
     [mColorPanel setDelegate:nil];
   }
 
@@ -82,25 +89,34 @@ HexStrToInt(NSString* str)
   mColorPicker = nullptr;
 
   [super dealloc];
 }
 @end
 
 NS_IMPL_ISUPPORTS1(nsColorPicker, nsIColorPicker)
 
+NSColorPanelWrapper* nsColorPicker::sColorPanelWrapper = nullptr;
+
 NS_IMETHODIMP
 nsColorPicker::Init(nsIDOMWindow* aParent, const nsAString& aTitle,
                     const nsAString& aInitialColor)
 {
+  MOZ_ASSERT(NS_IsMainThread(),
+      "Color pickers can only be opened from main thread currently");
   mTitle = aTitle;
   mColor = aInitialColor;
 
-  mColorPanel = [[NSColorPanelWrapper alloc] initWithPicker:this];
-
+  if (sColorPanelWrapper) {
+    // Update current wrapper to target the new input instead
+    [sColorPanelWrapper retarget:this];
+  } else {
+    // Create a brand new color panel wrapper
+    sColorPanelWrapper = [[NSColorPanelWrapper alloc] initWithPicker:this];
+  }
   return NS_OK;
 }
 
 /* static */ NSColor*
 nsColorPicker::GetNSColorFromHexString(const nsAString& aColor)
 {
   NSString* str = nsCocoaUtils::ToNSString(aColor);
 
@@ -125,33 +141,38 @@ nsColorPicker::GetHexStringFromNSColor(N
 }
 
 NS_IMETHODIMP
 nsColorPicker::Open(nsIColorPickerShownCallback* aCallback)
 {
   MOZ_ASSERT(aCallback);
   mCallback = aCallback;
 
-  [mColorPanel open:GetNSColorFromHexString(mColor)
+  [sColorPanelWrapper open:GetNSColorFromHexString(mColor)
               title:nsCocoaUtils::ToNSString(mTitle)];
 
   NS_ADDREF_THIS();
 
   return NS_OK;
 }
 
 void
 nsColorPicker::Update(NSColor* aColor)
 {
   GetHexStringFromNSColor(aColor, mColor);
   mCallback->Update(mColor);
 }
 
 void
-nsColorPicker::Done()
+nsColorPicker::DoneWithRetarget()
 {
   mCallback->Done(EmptyString());
   mCallback = nullptr;
-
-  [mColorPanel release];
-
   NS_RELEASE_THIS();
 }
+
+void
+nsColorPicker::Done()
+{
+  [sColorPanelWrapper release];
+  sColorPanelWrapper = nullptr;
+  DoneWithRetarget();
+}