bugGNUstep - Bugs: bug #25607, NSUndoManager sends...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #25607: NSUndoManager sends NSUndoManagerCheckpointNotification when opening a top-level undo group

Submitted by:  matt rice <ratmice>
Submitted on:  Tue 17 Feb 2009 12:20:20 PM UTC  
 
Category: Base/FoundationSeverity: 3 - Normal
Item Group: NoneStatus: Fixed
Privacy: PublicAssigned to: None
Open/Closed: Closed

(Jump to the original submission Jump to the original submission)

Wed 25 Feb 2009 04:02:19 PM UTC, comment #10:

OK. I think this is probably sorted now.
It seems that behavior varies depending on whether groupsByEvent is true.
If it is false, behavior seems to be what's intuitive.
It it's true the behavior is as follows:

When you call -beginUndoGrouping at the top level,
1. a top level group is implicitly created
2. the checkpoint notification is sent
3. another group is created.

When you call it an you are not at the top level, it sends the checkpoint notification and then creates a group.

Richard Frith-Macdonald <CaS>
Project Member
Wed 25 Feb 2009 12:45:46 PM UTC, comment #9:

Thanks, but I'm not so interested in "after" the beginUndoGrouping, I'm actually more interested in "during". I.e. what is the groupingLevel when the notification is posted. I hope tests I just added clarify what I mean...

Cheers,
David

David Ayers <ayers>
Project Member
Wed 25 Feb 2009 12:10:16 PM UTC, comment #8:

I looked at -groupingLevel while investigating this and I recall that ...

before calling -beginUndoGrouping it reports zero
after calling -beginUndoGrouping it reports two
after calling -endUndoGrouping it reports one
after an extra call to -endUndoGrouping it reports zero

To be absolutely sure, I just added a few testcases to NSUndoManager/NSUndoManager_notifications.m and can confirm that all the tests in this file pass on may MacOS-X 10.5 system using the Apple Foundation.

Richard Frith-Macdonald <CaS>
Project Member
Wed 25 Feb 2009 11:58:03 AM UTC, comment #7:

OK. So now 2 groups are created... that is very unfortunate and unexpected.

We would like to ignore any checkpoint notification during the top level beginUndoGrouping. It also seems strange that the implicit grouping will never be matched with a endUndoGroup.

Could I ask you validate what -groupingLevel reports during the processing of check point notification (especially the first two) when the manager groups by events?

David Ayers <ayers>
Project Member
Wed 25 Feb 2009 09:59:42 AM UTC, comment #6:

Thanks ... behavior reverted to match OSX 10.5

Richard Frith-Macdonald <CaS>
Project Member
Wed 25 Feb 2009 08:41:00 AM UTC, comment #5:

If the behavior changed then we need to work around it in EOControl. I've committed a fix in GDL2 that should be safe for the new Cocoa (previous GNUstep) behavior (tested with the base-1_16_3 release tag).

So therefore feel free to revert the change and mark this as invalid, Richard.

Cheers,
David

PS: If you don't get around to it, I'll do it this evening.

David Ayers <ayers>
Project Member
Sat 21 Feb 2009 05:27:36 PM UTC, comment #4:

hmmm, for the record i was looking here

http://developer.apple.com/DOCUMENTATION/Cocoa/Conceptual/UndoArchitecture/Tasks/UsingUndoNotif.html#//apple_ref/doc/uid/20000210

where it says

"whenever an NSUndoManager opens or closes a new undo group (except when it opens a top-level group)"

the actual API reference has a different wording which makes me believe that the API may have changed.

http://developer.apple.com/documentation/Cocoa/Reference/Foundation/Classes/NSUndoManager_Class/Reference/Reference.html#//apple_ref/occ/instm/NSUndoManager/beginUndoGrouping

"This method posts an NSUndoManagerCheckpointNotification unless a top-level undo is in progress."

that top documentation is also rather old, but is also reflected in the webobjects documentation and is the behaviour that gdl2 currently targets, anyhow if it doesn't match the foundation behaviour i suppose we'll have to figure something out....

matt rice <ratmice>
Project Member
Sat 21 Feb 2009 02:34:16 PM UTC, comment #3:

I'm reopening this because ... I ran the regression tests for NSUndoManager and it reported the effect of the change as an error (that makes sense since the test was designed to check the existing behavior). However, when I ran the same test on MacOS-X it also reported the new behavior as an error ... it seems that MacOS-X sends an NSUndoManagerCheckpointNotification on -beginGroup even if it is at the top level, or something else odd is going on.

Richard Frith-Macdonald <CaS>
Project Member
Tue 17 Feb 2009 12:56:11 PM UTC, comment #2:

yep, thanks

matt rice <ratmice>
Project Member
Tue 17 Feb 2009 12:49:20 PM UTC, comment #1:

I changed the code to refrain from posting the notification when there is no existing group (_group is nil).
Does that fix the problem?

Richard Frith-Macdonald <CaS>
Project Member
Tue 17 Feb 2009 12:20:20 PM UTC, original submission:

according to the docs it should be sent except during undo, or when opening a top-level undo group.

this appears to have triggered bug #25587

matt rice <ratmice>
Project Member

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by ayers (Posted a comment)
  • -unavailable- added by CaS (Posted a comment)
  • -unavailable- added by ratmice (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 25 Feb 2009 09:59:42 AM UTCCaSStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Sat 21 Feb 2009 02:34:16 PM UTCCaSStatusFixed=>None
      Open/ClosedClosed=>Open
    Tue 17 Feb 2009 12:56:11 PM UTCratmiceOpen/ClosedIn Test=>Closed
    Tue 17 Feb 2009 12:49:20 PM UTCCaSStatusNone=>Fixed
      Open/ClosedOpen=>In Test

    Back to the top


    Powered by Savane 3.1-cleanup1