bug-gnustep
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH (revised)] -setTarget: and -setAction of NSImageView


From: Kazunobu Kuriyama
Subject: Re: [PATCH (revised)] -setTarget: and -setAction of NSImageView
Date: Mon, 29 Dec 2003 14:51:58 +0900
User-agent: Mozilla/5.0 (X11; U; Linux i686; ja-JP; rv:1.4) Gecko/20030624 Netscape/7.1

Hi,

In a word, your comment is just "broader point of view" I've
been seeking for since this thread began.  Now I'm in favor
of the idea that NSImageCell should be a direct subclass of
NSCell.  I can't be against it any more.

In particular, what is written in the header file is definitely
decisive. (I was so stupid that I failed to ask about it.
If I asked it from the beginning, it would make the discussion
fairly shorter. :)

It is header files that neither programmers nor compilers
can't be against!

So, attached is an integrated patch which was made against the -gui
directory in the latest CVS.  Of course, modification was done
along with the idea mentioned above.

Could someone please check it and judge whether it is acceptable
or not?

Thanks a lot.

- Kazunobu Kuriyama


Gregory John Casamento wrote:

Kazunobu,

See below...

--- Kazunobu Kuriyama <kazunobu.kuriyama@nifty.com> wrote:

Hi,

Gregory John Casamento wrote:


<... my assertion from previous email about maintaining compatibility ...>

In addition to the comment above, if someone could give another
one on how an implementation using NSActionCell is technically
inconvenient, I think it would be definitely decisive.


It's inconvenient for a number of reasons:
------------------------------------------
1) It is in conflict with the documented hierarchy for this class (in both the
docs and in the header NSImageCell.h).
2) It will complicate decoding of previous versions of this class from .gorm
files since you will need to call the initWithCoder for NSCell instead of the
one for NSActionCell for all older versions of the class.   It is *NOT*
acceptable at all to simply allow the older .gorm files to break.
As the maintainer of Gorm, the encoding of the classes in both gui and base are
critical to .gorm files and are under my purview.

The difference of the responsibilities between NSCell and
NSActionCell is so obvious that Apple's engineers should
know it. Nonetheless, they decided to make NSImageCell a
subclass of NSCell (assuming the present document is still
correct.). Because the decision appears utterly illogical
to me, I think it is possible that the present document is
incorrect.  If I knew the techical reason of the decision,
I could easily withdraw my first view on the issue, i.e.
use of NSActionCell.


I don't know why they did it that way either, but lacking that I don't think
that not knowing the reason is sufficient cause for changing the parent class
especially when it's documented this way and it's in the headers.



Before applying any patch, we should also verify the behavior of MOSX

against

GNUstep.

Two questions should be answered before we proceed:
1) Does NSImageCell need to have an implementation for the target/action
behaviour? (i.e. is the currently situation acceptable?)


As pointed out by Alexander Malmberg, this is clear from
the documentation found at
http://developer.apple.com/documentation/Cocoa/Conceptual/ImageView/. For your convenience, I quote part of the article here:

<quote>
Using an Image View to Specify an Image
----------------------------------------
You can use an image view as an image well, which lets a user specify
an image by dragging an image to it. Just use setEditable: with an
argument of YES.  When the user drags an image to the image view,
the image view replaces its old image and sends its action to its
target.
<end of quote>

However, according to Fred Kiefer's comment, this is not included
in the OpenStep specs.


True, NSImageCell was not present under the OpenStep spec.  GNUstep isn't,
however, limited to the OpenStep spec and should not take liberties with the
classes which aren't specified there.

As I said before, we should follow the MOSX Cocoa Documentation.


2) Do we know what the behaviour is on MOSX?


Unfortunately, I have no access to MOSX.  If there's someone having
access to MOSX, I would appreciate it very much if she/he could try
a sample program to see what happens on the MOSX with NSImageView
having target/action.



I do have access to MOSX. :)  It is in the MOSX documentation for Objective-C
and Java with NSCell as the superclass.  It is also in the header as a subclass
of NSCell.  The idea that it's a "mistake in the docs" is soundly defeated
here.


The sample program is in CocoaProgramming-20021010/Chapter 9/Image Viewer 2
in a freely downloadable compressed file.  You can download it from
http://www.cocoaprogramming.net.  -setTarget: and -setAction methods
used with an NSImageView can be found in an init method of MYDocument.m.

Needless to say, if the program is linked against GNUstep, it terminates
immediately after it begins running, due to an exception raised
from either of the methods.


Well, that sounds like a problem which needs to be fixed, but changing the
class hierarchy in a way which is incompatible with MOSX isn't the answer.


Thanks.

- Kazunobu Kuriyama


Later, GJC

=====
Gregory John Casamento -- CEO/President Open Logic Corp.
-- bheron on #gnustep, #linuxstep, & #gormtalk ---------------- Please sign the petition against software patents at: http://www.petitiononline.com/pasp01/petition.html -- Main Developer of Gorm (featured in April Linux Journal) ---

__________________________________
Do you Yahoo!?
New Yahoo! Photos - easier uploading and sharing.
http://photos.yahoo.com/

2003-12-29 Kazunobu Kuriyama <kazunobu.kuriyama@nifty.com>

        * Headers/AppKit/NSImageCell.h: Add new ivars: _tag, _target,
        _action, and _control_view.  Redeclare methods relevant to
        target/action.
        * Source/NSImageCell.m (+initialize): Set version to 2 from 1.
        (-drawInteriorWithFrame:inView:): Set _control_view to the second
        parameter.
        (-action): Overriden.
        (-setAction:): Overriden.
        (-target): Overridden.
        (-setTarget:): Overridden.
        (-tag): Overridden.
        (-setTag:): Overridden.
        (-controlView): Overridden.
        (-encodeWithCoder:): Modified so that it can deals with new ivars.
        (-initWithCoder:): Modified so that it can deals with new ivars.
        * Source/NSImageView.m (-performDragOperation:): Let target invoke
        action when new image is set.  Add a check so that new image is set
        only if the view is editable.

diff -Naru gui/Headers/AppKit/NSImageCell.h gui.rev/Headers/AppKit/NSImageCell.h
--- gui/Headers/AppKit/NSImageCell.h    2003-12-29 13:57:42.000000000 +0900
+++ gui.rev/Headers/AppKit/NSImageCell.h        2003-12-29 14:10:59.000000000 
+0900
@@ -64,6 +64,12 @@
   NSImageFrameStyle _frameStyle;
   NSImageScaling _imageScaling;
   NSSize _original_image_size;
+
+  // [NSImageCell version] >= 2
+  int _tag;
+  id _target;
+  SEL _action;
+  id _control_view;
 }
 
 //
@@ -80,6 +86,20 @@
 - (NSImageFrameStyle) imageFrameStyle;
 - (void) setImageFrameStyle: (NSImageFrameStyle)aFrameStyle;
 
+//
+// Target and Action
+//
+// N.B: These methods are overridden so that the instances doesn't raise an
+//      exception when they receive a target or action message.
+//
+- (SEL) action;
+- (void) setAction: (SEL)aSelector;
+- (id) target;
+- (void) setTarget: (id)anObject;
+- (int) tag;
+- (void) setTag: (int)anInt;
+- (NSView *) controlView;
+
 @end
 
 #endif // _GNUstep_H_NSImageCell
diff -Naru gui/Source/NSImageCell.m gui.rev/Source/NSImageCell.m
--- gui/Source/NSImageCell.m    2003-12-29 13:57:43.000000000 +0900
+++ gui.rev/Source/NSImageCell.m        2003-12-29 14:01:32.000000000 +0900
@@ -41,7 +41,7 @@
 {
   if (self == [NSImageCell class])
     {
-      [self setVersion: 1];
+      [self setVersion: 2];
     }
 }
 
@@ -300,6 +300,9 @@
   // draw!
   [_cell_image compositeToPoint: position operation: NSCompositeSourceOver];
 
+  if (_control_view != controlView)
+    _control_view = controlView;
+
   if (_cell.shows_first_responder)
     NSDottedFrameRect(cellFrame);
 }
@@ -360,6 +363,44 @@
 }
 
 //
+//  Target and Action
+//
+- (SEL) action
+{
+  return _action;
+}
+
+- (void) setAction: (SEL)aSelector
+{
+  _action = aSelector;
+}
+
+- (id) target
+{
+  return _target;
+}
+
+- (void) setTarget: (id)anObject
+{
+  _target = anObject;
+}
+
+- (int) tag
+{
+  return _tag;
+}
+
+- (void) setTag: (int)anInt
+{
+  _tag = anInt;
+}
+
+- (NSView *) controlView
+{
+  return _control_view;
+}
+
+//
 // NSCoding protocol
 //
 - (void) encodeWithCoder: (NSCoder *)aCoder
@@ -370,6 +411,11 @@
   [aCoder encodeValueOfObjCType: @encode(NSImageFrameStyle) at: &_frameStyle];
   [aCoder encodeValueOfObjCType: @encode(NSImageScaling) at: &_imageScaling];
   [aCoder encodeSize: _original_image_size];
+
+  [aCoder encodeValueOfObjCType: @encode(int) at: &_tag];
+  [aCoder encodeConditionalObject: _target];
+  [aCoder encodeValueOfObjCType: @encode(SEL) at: &_action];
+  [aCoder encodeConditionalObject: _control_view];
 }
 
 - (id) initWithCoder: (NSCoder *)aDecoder
@@ -380,6 +426,15 @@
   [aDecoder decodeValueOfObjCType: @encode(NSImageFrameStyle) at: 
&_frameStyle];
   [aDecoder decodeValueOfObjCType: @encode(NSImageScaling) at: &_imageScaling];
   _original_image_size = [aDecoder decodeSize];
+
+  if ([aDecoder versionForClassName: @"NSImageCell"] >= 2)
+    {
+      [aDecoder decodeValueOfObjCType: @encode(int) at: &_tag];
+      _target = [aDecoder decodeObject];
+      [aDecoder decodeValueOfObjCType: @encode(SEL) at: &_action];
+      _control_view = [aDecoder decodeObject];
+    }
+
   return self;
 }
 
diff -Naru gui/Source/NSImageView.m gui.rev/Source/NSImageView.m
--- gui/Source/NSImageView.m    2003-12-29 13:57:43.000000000 +0900
+++ gui.rev/Source/NSImageView.m        2003-12-29 14:02:21.000000000 +0900
@@ -184,18 +184,30 @@
 - (BOOL) performDragOperation: (id <NSDraggingInfo>)sender
 {
   NSImage *image;
+  BOOL retval;
 
-  image = [[NSImage alloc] initWithPasteboard: [sender draggingPasteboard]];
-  if (image == nil)
+  if ([sender draggingSource] == self || ![self isEditable])
     {
-      return NO;
+      retval = NO;
     }
-  else 
+  else
     {
-      [self setImage: image];
-      RELEASE(image);
-      return YES;
+      image = [[NSImage alloc] initWithPasteboard: [sender 
draggingPasteboard]];
+      if (image == nil)
+       {
+         retval = NO;
+       }
+      else 
+       {
+         [self setImage: image];
+         [self sendAction: [[self cell] action]
+                       to: [[self cell] target]];
+         retval = YES;
+         RELEASE(image);
+       }
     }
+
+  return retval;
 }
 
 - (void) concludeDragOperation: (id <NSDraggingInfo>)sender

reply via email to

[Prev in Thread] Current Thread [Next in Thread]