When the editor gets focus check for changes

NEW

Status

SeaMonkey
Composer
--
enhancement
18 years ago
5 years ago

People

(Reporter: Neil Marshall, Assigned: neil@parkwaycc.co.uk)

Tracking

({helpwanted, polish})

Trunk
helpwanted, polish

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

18 years ago
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.

Updated

18 years ago
Keywords: 4xp
Priority: P3 → P5
Hardware: PC → All
Whiteboard: help wanted
Target Milestone: M20

Updated

18 years ago
Status: NEW → ASSIGNED

Updated

18 years ago
Keywords: helpwanted
Whiteboard: help wanted

Comment 1

17 years ago
reassign my M20 bugs to beppe so the editor rfe's are all in one spot :-)
Assignee: brade → beppe
Status: ASSIGNED → NEW

Updated

17 years ago
Status: NEW → ASSIGNED

Comment 2

17 years ago
moving to future milestone
Target Milestone: M20 → Future

Comment 3

17 years ago
moving back to previous owner
Assignee: beppe → brade
Status: ASSIGNED → NEW

Updated

17 years ago
Status: NEW → ASSIGNED

Comment 4

16 years ago
spam composer change
Component: Editor: Core → Editor: Composer

Updated

15 years ago
Keywords: polish

Comment 5

15 years ago
*** Bug 37089 has been marked as a duplicate of this bug. ***

Comment 6

15 years ago
-->
Assignee: brade → composer
Status: ASSIGNED → NEW
(Assignee)

Comment 7

15 years ago
Created attachment 111398 [details]
Screen shot
(Assignee)

Comment 8

15 years ago
Created attachment 111401 [details] [diff] [review]
Proposed patch
(Assignee)

Updated

15 years ago
Attachment #111401 - Flags: superreview?(sfraser)
Attachment #111401 - Flags: review?(brade)

Comment 9

15 years ago
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!
Attachment #111401 - Flags: superreview?(sfraser)
Attachment #111401 - Flags: superreview-
Attachment #111401 - Flags: review?(brade)
Attachment #111401 - Flags: review-

Comment 10

15 years ago
-->Neil
Assignee: composer → neil
(Assignee)

Comment 11

15 years ago
Created attachment 111498 [details] [diff] [review]
Addressed brade's comments

Thanks for those comments. I suffered from copy&paste syndrome :-(
Attachment #111401 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #111498 - Flags: superreview?(sfraser)
Attachment #111498 - Flags: review?(brade)

Comment 12

15 years ago
Comment on attachment 111498 [details] [diff] [review]
Addressed brade's comments

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

r=brade
Attachment #111498 - Flags: review?(brade) → review+

Comment 13

15 years ago
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.
(Assignee)

Comment 14

15 years ago
... although that can easily be changed too.

Comment 15

15 years ago
Window title wording looks fine.

Comment 16

15 years ago
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

15 years ago
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...."
(Assignee)

Comment 18

15 years ago
Created attachment 112280 [details] [diff] [review]
Addressed sfraser's comments
Attachment #111498 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #112280 - Flags: superreview?(sfraser)
Attachment #112280 - Flags: review?(brade)

Comment 19

15 years ago
Comment on attachment 112280 [details] [diff] [review]
Addressed sfraser's comments

r=brade
I like Robin's wording.
Attachment #112280 - Flags: review?(brade) → review+

Comment 20

15 years ago
+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.
(Assignee)

Comment 21

15 years ago
Shall I change the Revert message to use "Discard" while I'm at it?
(Assignee)

Comment 22

15 years ago
The button title is defined as "Abandon" somewhere else...
won't that be confusing?

Comment 23

15 years ago
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.
(Assignee)

Comment 24

15 years ago
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.
(Assignee)

Comment 25

15 years ago
Created attachment 112519 [details] [diff] [review]
Discarding use of Abandon
Attachment #112280 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #112519 - Flags: superreview?(sfraser)
Attachment #112519 - Flags: review?(brade)

Comment 26

15 years ago
Comment on attachment 112519 [details] [diff] [review]
Discarding use of Abandon

r=brade
Attachment #112519 - Flags: review?(brade) → review+

Comment 27

15 years ago
+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

15 years ago
Comment on attachment 111498 [details] [diff] [review]
Addressed brade's comments

old patch
Attachment #111498 - Flags: superreview?(sfraser) → superreview-

Comment 29

15 years ago
Comment on attachment 112280 [details] [diff] [review]
Addressed sfraser's comments

old patch
Attachment #112280 - Flags: superreview?(sfraser) → superreview-

Comment 30

14 years ago
Neil--can you get someone to review this patch (is it still valid)?
Target Milestone: Future → mozilla1.4beta
(Assignee)

Comment 31

14 years ago
brade, I got stuck trying to think of a solution to comment 27.
Product: Browser → Seamonkey
Priority: P5 → --
QA Contact: sujay → composer
Target Milestone: mozilla1.4beta → ---

Comment 32

9 years ago
Comment on attachment 112519 [details] [diff] [review]
Discarding use of Abandon

Clearing old request
Attachment #112519 - Flags: superreview?(sfraser_bugs) → superreview?
Comment on attachment 112519 [details] [diff] [review]
Discarding use of Abandon


Please, re-request sr with a requestee :-|
Attachment #112519 - Flags: superreview?

Comment 34

5 years ago
(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? :)
(Assignee)

Comment 35

5 years ago
(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...
You need to log in before you can comment on or make changes to this bug.