Do I need to override revertToSaved ?

I'm creating a MacOS document-based app in Swift. When running the app, if I make modifications to the document, and then select "Revert to Saved", I get a dialog asking me if I want to revert the document, losing current changes, but if I click on "Revert", nothing happens. The document keeps its changes.


Furthermore, after clicking Revert, the document behaves as if there are no changes -- the titlebar icon is not grey; and the window closes without asking to save changes (I'm not using Autosave). It's like ChangeCount or isDocumentEdited have been reset.


If I don't select Revert, then closing the document's window with unsaved changes brings a dialog asking to save.
Apple's documentation for .revertToSaved reads as follows:

The default implementation of this method presents an alert dialog giving the user the opportunity to cancel the operation. If the user chooses to continue, the method ensures that any editor registered using the Cocoa Bindings

NSEditorRegistration
informal protocol has discarded its changes and then invokes
revert(toContentsOf:ofType:)
. If that returns
false
, the method presents the error to the user in an document-modal alert dialog.


This suggests to me that it should work without needing to be overridden.


Any idea why this is happening? Do I need to override the revert method to load the data back from disk?
I've not yet sorted out Undo for the app, which might be relevant. But I am using ChangeCount to check whether edits have been made.

Answered by DelawareMathGuy in 385414022

hi,


after reading John's reply and then your code above, i'm sorry now to say that the advice i was offering was only correct if you were following MVC/iOS paradigm. i assumed you were. my apologies for not fully finishing the code.


John is right on three fronts:


(1) MacOS is becoming more iOS-like; the notions of an NSViewController even being in the responder chain and having viewWillAppear() and viewDidAppear() were only introduced maybe 4 years ago.


(2) your code is setting up its views to have direct references to the things they draw (along with some parameters). when the NSDocument is reverted, you're not telling the views that their references must be updated first before the views are redrawn.


(3) you need to set up references for the NSViews outside of viewDidAppear(). in fact, i don't see that you even want to touch viewDidAppear(). then you can call that code both upon startup and when revert happens.


so, based on your question and seeing your most recent code above, John's asking you to rewrite this way:



override func viewDidLoad() { 
  super.viewDidLoad() 
  NotificationCenter.default.addObserver(self, selector: #selector(handleDocumentReverted), name: .documentReverted, object: nil) 
  loadViewParameters() 
} 


func loadViewParameters() { 
  let theThumbnailSize = CGSize(width: 200, height: 200) 
  self.thePDFView?.document = document?.thePDFDocument 
  // self.thePDFView?.autoScales = true 
  self.thePDFView?.displayMode = theDisplayMode 
  self.thePDFView?.displaysAsBook = bookState 


  self.theThumbnailView?.pdfView = self.thePDFView 
  self.theThumbnailView?.thumbnailSize = theThumbnailSize 
} 


@objc func handleDocumentReverted(_ note: Notification) { 
  // i mentioned that you may not have to check directly in some cases
  // whether you're responding to the right document's revert in these 2 lines:
  // let sendingDocument = note.object as? Document 
  // guard sendingDocument == document else { return }
  loadViewParameters()
  thePDFView.setNeedsDisplay(thePDFView.bounds) 
  theThumbnailView.setNeedsDisplay(theThumbnailView.bounds) 
}



hope that helps,

DMG

OK. I found your github project. I don't know how to use the app though. I don't see any way to modify a PDF document to test saving and reverting.


Right off the bat, you are going to be in for a ride. You are trying to write a version of Preview that does not use modern NSDocument behaviour. That is a huge plot point. Virtually all advice you are likely to get regarding NSDocument is going to be telling you how to get modern NSDocument behaviour working properly. Getting NSDocument to work properly can be a challenge. Trying to get NSDocument to do something that it was not designed to do is going to be much more difficult.


That detail aside, I can tell you what the problem is. You are initializing your PDF view in viewDidAppear. That isn't getting called as a side effect of revert. Therefore, the PDF view is still displaying the old PDF document. You may have reset the PDFDocument in your document object, but the reference to the PDF document that the PDF view holds is still the old one.


I think part of the problem may be the iOS-style ViewController logic you are using. I think I had decided against mentioning this aspect in an effort to avoid discouragement. But I guess I should say something. This architecture is something that Apple ported to the Mac as a way to make iOS developers feel more comfortable. Apple is slowly deprecating all of macOS, regardless of what any Apple employee may say about the matter. iOS doesn't have bindings, so the UI flow is completely different there. There are lots of attractive-looking hooks like viewDidAppear, but you have to be careful with them. It does what it says it says. But It probably doesn't get called after revert. Since it is part of a delicate architecture much like NSDocument, you shouldn't force the issue by trying to call it manually. I can't say for sure when viewDidAppear is or is not called. I don't use the iOS ViewController style logic myself. But I generally avoid making any such assumptions about what Apple's code might do.


You may be able to just assign a new document to the view. The notifications listed in PDFView seem to suggest that will work. I think the notification might be the right approach here. When you get a notification that your underlying data has changed, just refresh your PDF view by changing its document. You can have your viewDidAppear method call the same update method that your notification handler calls.

John, Many thanks for this. (You should be able to Rotate and Delete pages in the app.)


Can you clarify "You are initializing your PDF view in viewDidAppear. That isn't getting called as a side effect of revert."?


I tried moving everything from viewDidAppear to viewDidLoad, but I just got a grey window.


I've no objection to using NSDocument 'the right way', I'm just using it as minimally as I can to get it working. As the app grows, no doubt that will change. I've read Apple's pages on NSDocument, which describes a text doc, and I can't always see how it applies.


I know it's a "long-term project": but I've been filing BRs and ERs about Preview for 10 years, so... 😁


  // OVERRIDES
    override func viewDidLoad() {
        super.viewDidLoad()
        NotificationCenter.default.addObserver(self, selector: #selector(handleDocumentReverted), name: .documentReverted, object: nil)
        // Do any additional setup after loading the view.
    }
   
    override func viewDidAppear() {
       
        let theThumbnailSize = CGSize(width: 200, height: 200)
        self.thePDFView?.document = document?.thePDFDocument
        // self.thePDFView?.autoScales = true
        self.thePDFView?.displayMode = theDisplayMode
        self.thePDFView?.displaysAsBook = bookState

        self.theThumbnailView?.pdfView = self.thePDFView
        self.theThumbnailView?.thumbnailSize = theThumbnailSize
    }
   

    override var representedObject: Any? {
        didSet {
        // Update the view, if already loaded.
        }
    }
Accepted Answer

hi,


after reading John's reply and then your code above, i'm sorry now to say that the advice i was offering was only correct if you were following MVC/iOS paradigm. i assumed you were. my apologies for not fully finishing the code.


John is right on three fronts:


(1) MacOS is becoming more iOS-like; the notions of an NSViewController even being in the responder chain and having viewWillAppear() and viewDidAppear() were only introduced maybe 4 years ago.


(2) your code is setting up its views to have direct references to the things they draw (along with some parameters). when the NSDocument is reverted, you're not telling the views that their references must be updated first before the views are redrawn.


(3) you need to set up references for the NSViews outside of viewDidAppear(). in fact, i don't see that you even want to touch viewDidAppear(). then you can call that code both upon startup and when revert happens.


so, based on your question and seeing your most recent code above, John's asking you to rewrite this way:



override func viewDidLoad() { 
  super.viewDidLoad() 
  NotificationCenter.default.addObserver(self, selector: #selector(handleDocumentReverted), name: .documentReverted, object: nil) 
  loadViewParameters() 
} 


func loadViewParameters() { 
  let theThumbnailSize = CGSize(width: 200, height: 200) 
  self.thePDFView?.document = document?.thePDFDocument 
  // self.thePDFView?.autoScales = true 
  self.thePDFView?.displayMode = theDisplayMode 
  self.thePDFView?.displaysAsBook = bookState 


  self.theThumbnailView?.pdfView = self.thePDFView 
  self.theThumbnailView?.thumbnailSize = theThumbnailSize 
} 


@objc func handleDocumentReverted(_ note: Notification) { 
  // i mentioned that you may not have to check directly in some cases
  // whether you're responding to the right document's revert in these 2 lines:
  // let sendingDocument = note.object as? Document 
  // guard sendingDocument == document else { return }
  loadViewParameters()
  thePDFView.setNeedsDisplay(thePDFView.bounds) 
  theThumbnailView.setNeedsDisplay(theThumbnailView.bounds) 
}



hope that helps,

DMG

DMG, you have nothing to apologise for! If there's any fault, it's in my not supplying enough relevant code to start with. (Though, of course, knowing what's relevant is half the battle.)


Your code worked with one alteration: As is, I got a blank grey window at launch. By including loadViewParameters in viewWillAppear() as well as in the Revert Handler, it works perfectly! Nothing was needed in viewDidLoad except the Notification. (In fact, the super isn't even necessary -- according to Apple, the default implementation does nothing.)


So I'm going to mark it as the correct answer.


Right, onto Undo! 😕

DMG's reply is very nice. I think the viewDidLoad() method was used just because you mentioned trying that most recently. One of the most tricky things to deal with is lifecycle. Consider these events:


app init

open doc

create doc

init doc

read nib

read data

layout views

display views

redraw views

modify data

modify views

revert data

revert views

save data

close window

close doc

dealloc doc

quit app


This list is incomplete. There are different overrides for each of these events. Sometimes there are multiple potential overrides you could do. These events are not necessarily in the proper order. They can sometimes occur in different orders depending on the circumstances. And yet, certain events must always come before certain other events. Sometimes, certain events don't occur. For example, if you quit the app without closing documents first, the document dealloc is never called, so be careful what you do in there.


At least undo is much easier and more pleasant to work with. I had one app with extensive custom undo support. I forget which app it was. Ah, I remember! I was stupid enough to write an RSS reader once. You can use invocations to record undo operations. For each operation, identify an operation that would undo it. Then, do something like this:


[[undoManager prepareWithInvocationTarget: self]

markArticles: articles read: !read];


The "markArticles:read:" method is one of mine. When the user performs undo, it just calls this method with the given parameters. In your case, if the user is performing a rotate right 90deg, you issue a prepareWithInvocationTarget that does a rotate left 90deg.


I'm not sure how Swift will handle that. Maybe it uses blocks now. The date on my source file is 2012. 🙂

Everything in AtOmXpLuS.CoM

Do I need to override revertToSaved ?
 
 
Q