Open Bug 27820 Opened 25 years ago Updated 13 years ago

When the editor gets focus check for changes

Categories

(SeaMonkey :: Composer, enhancement)

Product:
SeaMonkey
Component:
Composer
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

People

(Reporter: marshall, Assigned: neil)

References

Details

(Keywords: helpwanted, polish)

Attachments

(2 files, 3 obsolete files)

Screen shot
22.63 KB, image/png
Details
Discarding use of Abandon
7.20 KB, patch
Brade
: review+
Details | Diff | Splinter Review
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.
Keywords: 4xp
Priority: P3 → P5
Hardware: PC → All
Whiteboard: help wanted
Target Milestone: M20
Status: NEW → ASSIGNED
Keywords: helpwanted
Whiteboard: help wanted
reassign my M20 bugs to beppe so the editor rfe's are all in one spot :-)
Assignee: brade → beppe
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
moving to future milestone
Target Milestone: M20 → Future
moving back to previous owner
Assignee: beppe → brade
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
spam composer change
Component: Editor: Core → Editor: Composer
Keywords: polish
*** Bug 37089 has been marked as a duplicate of this bug. ***
-->
Assignee: brade → composer
Status: ASSIGNED → NEW
Attached image Screen shot —
Attached patch Proposed patch (obsolete) — — Splinter Review
Attachment #111401 - Flags: superreview?(sfraser)
Attachment #111401 - Flags: review?(brade)
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-
-->Neil
Assignee: composer → neil
Attached patch Addressed brade's comments (obsolete) — — Splinter Review
Thanks for those comments. I suffered from copy&paste syndrome :-(
Attachment #111401 - Attachment is obsolete: true
Attachment #111498 - Flags: superreview?(sfraser)
Attachment #111498 - Flags: review?(brade)
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+
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.
... although that can easily be changed too.
Window title wording looks fine.
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.
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...."
Attached patch Addressed sfraser's comments (obsolete) — — Splinter Review
Attachment #111498 - Attachment is obsolete: true
Attachment #112280 - Flags: superreview?(sfraser)
Attachment #112280 - Flags: review?(brade)
Comment on attachment 112280 [details] [diff] [review] Addressed sfraser's comments r=brade I like Robin's wording.
Attachment #112280 - Flags: review?(brade) → review+
+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.
Shall I change the Revert message to use "Discard" while I'm at it?
The button title is defined as "Abandon" somewhere else... won't that be confusing?
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.
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.
Attachment #112280 - Attachment is obsolete: true
Attachment #112519 - Flags: superreview?(sfraser)
Attachment #112519 - Flags: review?(brade)
Comment on attachment 112519 [details] [diff] [review] Discarding use of Abandon r=brade
Attachment #112519 - Flags: review?(brade) → review+
+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 on attachment 111498 [details] [diff] [review] Addressed brade's comments old patch
Attachment #111498 - Flags: superreview?(sfraser) → superreview-
Comment on attachment 112280 [details] [diff] [review] Addressed sfraser's comments old patch
Attachment #112280 - Flags: superreview?(sfraser) → superreview-
Neil--can you get someone to review this patch (is it still valid)?
Target Milestone: Future → mozilla1.4beta
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 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?
(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? :)
(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.

Attachment

General

Creator:
Created:
Updated:
Size: