Last Comment Bug 27820 - When the editor gets focus check for changes
: When the editor gets focus check for changes
Status: NEW
: helpwanted, polish
Product: SeaMonkey
Classification: Client Software
Component: Composer (show other bugs)
: Trunk
: All All
: -- enhancement with 3 votes (vote)
: ---
Assigned To: neil@parkwaycc.co.uk
:
Mentors:
: 37089 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2000-02-15 08:49 PST by Neil Marshall
Modified: 2012-07-14 09:32 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Screen shot (22.63 KB, image/png)
2003-01-13 07:56 PST, neil@parkwaycc.co.uk
no flags Details
Proposed patch (4.66 KB, patch)
2003-01-13 09:08 PST, neil@parkwaycc.co.uk
brade: review-
brade: superreview-
Details | Diff | Splinter Review
Addressed brade's comments (5.50 KB, patch)
2003-01-14 03:22 PST, neil@parkwaycc.co.uk
brade: review+
sfraser_bugs: superreview-
Details | Diff | Splinter Review
Addressed sfraser's comments (7.05 KB, patch)
2003-01-22 06:32 PST, neil@parkwaycc.co.uk
brade: review+
sfraser_bugs: superreview-
Details | Diff | Splinter Review
Discarding use of Abandon (7.20 KB, patch)
2003-01-24 06:52 PST, neil@parkwaycc.co.uk
brade: review+
Details | Diff | Splinter Review

Description Neil Marshall 2000-02-15 08:49:20 PST
When the editor gets focus it should check to see if the file has been modified. 
 If it has, bring up a dialog box and ask if the file should be reloaded.
Comment 1 Kathleen Brade 2000-03-31 06:13:49 PST
reassign my M20 bugs to beppe so the editor rfe's are all in one spot :-)
Comment 2 rubydoo123 2000-06-06 13:39:41 PDT
moving to future milestone
Comment 3 rubydoo123 2000-06-06 16:28:58 PDT
moving back to previous owner
Comment 4 Syd Logan 2001-09-12 13:27:56 PDT
spam composer change
Comment 5 Kathleen Brade 2002-10-09 18:08:00 PDT
*** Bug 37089 has been marked as a duplicate of this bug. ***
Comment 6 Kathleen Brade 2003-01-10 11:29:46 PST
-->
Comment 7 neil@parkwaycc.co.uk 2003-01-13 07:56:03 PST
Created attachment 111398 [details]
Screen shot
Comment 8 neil@parkwaycc.co.uk 2003-01-13 09:08:44 PST
Created attachment 111401 [details] [diff] [review]
Proposed patch
Comment 9 Kathleen Brade 2003-01-13 10:21:15 PST
Comment on attachment 111401 [details] [diff] [review]
Proposed patch

I don't see any changes to editorApplicationOverlay.js

SetSaveAndPublishUI isn't just setting UI now; add a comment to explain the
code you are adding.

Also, why return "" in SetSaveAndPublishUI?  just return.

RevertDocument seems poorly named since it's really prompting for the
reverting.  How about PromptRevertDocument?

Very cool progress!  Great work Neil!
Comment 10 Kathleen Brade 2003-01-13 10:21:55 PST
-->Neil
Comment 11 neil@parkwaycc.co.uk 2003-01-14 03:22:51 PST
Created attachment 111498 [details] [diff] [review]
Addressed brade's comments

Thanks for those comments. I suffered from copy&paste syndrome :-(
Comment 12 Kathleen Brade 2003-01-14 06:46:52 PST
Comment on attachment 111498 [details] [diff] [review]
Addressed brade's comments

+ModifiedCaption=Document Modified On Disk
I'd lower case "on"

r=brade
Comment 13 Kathleen Brade 2003-01-14 07:23:51 PST
cc Robin so she can provide a better window title (if desired).  Robin--there is
a screenshot attached as well as the patch with the string being added at the
very end of the patch (skip to bottom).  The string used for the prompt is the
same as the string used for the File > Revert menu item.
Comment 14 neil@parkwaycc.co.uk 2003-01-14 09:34:55 PST
... although that can easily be changed too.
Comment 15 robinf 2003-01-14 12:07:39 PST
Window title wording looks fine.
Comment 16 Simon Fraser 2003-01-16 12:19:13 PST
I don't like the wording of the dialog (as seen in the screenshot). I'd prefer
we have two separate wordings:
1. When doc has not been modified by composer since last save:
   "The document has been changed on disk by another application. Do you want
    to load the new version?"
                                        [Cancel]   [Load]

2. When the doc has changed on disk, and there are unsaved changes in
   Composer:
    "The document has been changed on disk by another application. Do you
     want to throw away your changes and load the new version?"
                                        [Cancel]   [Load]

Or something like that. I think the window title could also improve, since
"Document Modified on Disk" doesn't indicate to me that it was an external app
that changed it.
Comment 17 robinf 2003-01-21 10:27:15 PST
For window title, how about "Document Modified By Another Application".

>1. When doc has not been modified by composer since last save:
>   "The document has been changed on disk by another application. Do you want
>    to load the new version?"
>                                        [Cancel]   [Load]

OK.

>2. When the doc has changed on disk, and there are unsaved changes in
>   Composer:
>    "The document has been changed on disk by another application. Do you
>     want to throw away your changes and load the new version?"
>                                        [Cancel]   [Load]

Suggest changing "throw away" to "discard": "Do you want to discard your changes
and...."
Comment 18 neil@parkwaycc.co.uk 2003-01-22 06:32:25 PST
Created attachment 112280 [details] [diff] [review]
Addressed sfraser's comments
Comment 19 Kathleen Brade 2003-01-22 07:33:20 PST
Comment on attachment 112280 [details] [diff] [review]
Addressed sfraser's comments

r=brade
I like Robin's wording.
Comment 20 Simon Fraser 2003-01-22 09:46:30 PST
+LoadModified="%title%" has been changed on disk by another application. Abandon
unsaved changes and reload page?
 AbandonChanges=Abandon unsaved changes to "%title%" and reload page?

It was suggested to use "Discard", rather than "abandon". I agree.
Comment 21 neil@parkwaycc.co.uk 2003-01-23 03:15:55 PST
Shall I change the Revert message to use "Discard" while I'm at it?
Comment 22 neil@parkwaycc.co.uk 2003-01-23 08:58:07 PST
The button title is defined as "Abandon" somewhere else...
won't that be confusing?
Comment 23 robinf 2003-01-23 10:04:26 PST
Yes, we should also change "Abandon" to "Discard" in the Revert message. Not
sure if that change is within the scope of this bug, though.
Comment 24 neil@parkwaycc.co.uk 2003-01-23 12:51:57 PST
Sorry, my mistake, the button says "Revert", not "Abandon" or "Discard".

Anyway, I don't see the point of filing a new bug for the word change so I'll
just attach an updated patch here.
Comment 25 neil@parkwaycc.co.uk 2003-01-24 06:52:42 PST
Created attachment 112519 [details] [diff] [review]
Discarding use of Abandon
Comment 26 Kathleen Brade 2003-01-24 08:02:46 PST
Comment on attachment 112519 [details] [diff] [review]
Discarding use of Abandon

r=brade
Comment 27 Simon Fraser 2003-01-28 18:28:43 PST
+ReloadUnmodified="%title%" has been changed on disk by another application. Do
you want to load the new version?
+ReloadModified="%title%" has been changed on disk by another application.
Discard unsaved changes and reload page?

It would be nice to standardize terminology. I suggest the second one should say
"... Do you want to discard unsaved changes and load the new version?".

I also hesitate to add new globals to the editor JS, at a time when we're moving
towards being able to deal with > 1 editor in a window. We need some per-editor
data structure off of which the nsIFile and mod date can hang.
Comment 28 Simon Fraser 2003-02-12 09:54:07 PST
Comment on attachment 111498 [details] [diff] [review]
Addressed brade's comments

old patch
Comment 29 Simon Fraser 2003-02-12 09:54:32 PST
Comment on attachment 112280 [details] [diff] [review]
Addressed sfraser's comments

old patch
Comment 30 Kathleen Brade 2003-05-01 10:43:22 PDT
Neil--can you get someone to review this patch (is it still valid)?
Comment 31 neil@parkwaycc.co.uk 2003-05-01 13:06:42 PDT
brade, I got stuck trying to think of a solution to comment 27.
Comment 32 Simon Fraser 2008-12-21 23:24:34 PST
Comment on attachment 112519 [details] [diff] [review]
Discarding use of Abandon

Clearing old request
Comment 33 Serge Gautherie (:sgautherie) 2010-04-20 07:26:36 PDT
Comment on attachment 112519 [details] [diff] [review]
Discarding use of Abandon


Please, re-request sr with a requestee :-|
Comment 34 Phoenix 2012-07-14 07:29:55 PDT
(In reply to neil@parkwaycc.co.uk from comment #31)
> brade, I got stuck trying to think of a solution to comment 27.
And what do you think now, just 9 years after? :)
Comment 35 neil@parkwaycc.co.uk 2012-07-14 09:32:07 PDT
(In reply to Phoenix from comment #34)
> (In reply to comment #31)
> > brade, I got stuck trying to think of a solution to comment 27.
> And what do you think now, just 9 years after? :)
Well, I haven't seen any sign of moving to multiple editors in one window...

Note You need to log in before you can comment on or make changes to this bug.