Open Bug 401365 Opened 17 years ago Updated 12 years ago

Sync mail/ and mailnews/ <mailWindowOverlay.xul>

Categories

(SeaMonkey :: MailNews: Message Display, defect)

defect
Not set
minor

Tracking

(Not tracked)

ASSIGNED

People

(Reporter: sgautherie, Assigned: sgautherie)

References

(Depends on 1 open bug)

Details

Attachments

(4 files, 2 obsolete files)

* Whitespaces already removed from mail/ file.
* Identical code, but not at the same place.
* Then, investigate code which is different.
Attached patch (Av1-TB) whitespaces (obsolete) — Splinter Review
Assignee: mail → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #286403 - Flags: superreview?(mscott)
Attachment #286403 - Flags: review?(mscott)
Comment on attachment 286403 [details] [diff] [review]
(Av1-TB) whitespaces

(Wrong patch, sorry.)
Attachment #286403 - Attachment is obsolete: true
Attachment #286403 - Flags: superreview?(mscott)
Attachment #286403 - Flags: review?(mscott)
Attachment #286406 - Flags: superreview?(mscott)
Attachment #286406 - Flags: review?(mscott)
Attachment #286407 - Flags: superreview?(neil)
Attachment #286407 - Flags: review?(iann_bugzilla)
Comment on attachment 286407 [details] [diff] [review]
(Bv1-SM) whitespaces
[Checkin: comment 8]

>@@ -720,28 +720,28 @@
>         label="&folderContextProperties.label;"
>         accesskey="&folderContextProperties.accesskey;"
>         oncommand="MsgFolderProperties();"/>
>-  </popup> 
>+  </popup>
> 
>-  <popup id="messagePaneContext"   
>+  <popup id="messagePaneContext"
>      onpopupshowing="if (event.target != this) return true; gContextMenu = new nsContextMenu(this); return fillMessagePaneContextMenu();"
>      onpopuphiding="if (event.target == this) gContextMenu = null;">
>         <menuitem id="context-openlink"
>                   label="&openLinkCmd.label;"
>                   accesskey="&openLinkCmd.accesskey;"
>-                  oncommand="gContextMenu.openLink();"/> 
>+                  oncommand="gContextMenu.openLink();"/>
>         <menuitem id="context-openlinkintab"
> 	          label="&openLinkCmdInTab.label;"
>                   accesskey="&openLinkCmdInTab.accesskey;"
>                   oncommand="gContextMenu.openLinkInTab(event.shiftKey);"/>
>-        <menuseparator id="messagePaneContext-sep-link"/>
>-        <menuitem id="context-selectall"
>-                  label="&selectAllCmd.label;"
>-                  accesskey="&selectAllCmd.accesskey;"
>-                  command="cmd_selectAll"/>
>-        <menuitem id="context-copy"
>-                  label="&copyCmd.label;"
>-                  accesskey="&copyCmd.accesskey;"
>-                  command="cmd_copy"/>
>+    <menuseparator id="messagePaneContext-sep-link"/>
>+    <menuitem id="context-selectall"
>+              label="&selectAllCmd.label;"
>+              accesskey="&selectAllCmd.accesskey;"
>+              command="cmd_selectAll"/>
>+    <menuitem id="context-copy"
>+              label="&copyCmd.label;"
>+              accesskey="&copyCmd.accesskey;"
>+              command="cmd_copy"/>
Don't you need to reindent the first two menuitems too?

>@@ -978,10 +978,10 @@
>                   accesskey="&viewImageCmd.accesskey;"
>                   oncommand="gContextMenu.viewImage();"/>
>         <menuseparator id="messagePaneContext-sep-image"/>
>-        <menuitem id="context-composeemailto"
>-                  label="&SendMailTo.label;"
>-                  accesskey="&SendMailTo.accesskey;"
>-                  oncommand="SendMailTo(gContextMenu.getEmail());"/>
>+    <menuitem id="context-composeemailto"
>+              label="&SendMailTo.label;"
>+              accesskey="&SendMailTo.accesskey;"
>+              oncommand="SendMailTo(gContextMenu.getEmail());"/>
>         <menuitem id="context-createfilterfrom"
>                   label="&CreateFilterFrom.label;"
>                   accesskey="&CreateFilterFrom.accesskey;"
>@@ -990,10 +990,10 @@
>                   label="&AddToAddressBook.label;"
>                   accesskey="&AddToAddressBook.accesskey;"
>                   oncommand="AddEmailToAddressBook(gContextMenu.getEmail(), gContextMenu.linkText());"/>
>-        <menuitem id="context-copyemail"
>-                  label="&copyEmailCmd.label;"
>-                  accesskey="&copyEmailCmd.accesskey;"
>-                  oncommand="gContextMenu.copyEmail();"/>
>+    <menuitem id="context-copyemail"
>+              label="&copyEmailCmd.label;"
>+              accesskey="&copyEmailCmd.accesskey;"
>+              oncommand="gContextMenu.copyEmail();"/>
>         <menuitem id="context-copylink"
>                   label="&copyLinkCmd.label;"
>                   accesskey="&copyLinkCmd.accesskey;"
>@@ -1002,22 +1002,21 @@
>                   label="&copyImageCmd.label;"
>                   accesskey="&copyImageCmd.accesskey;"
>                   command="cmd_copyImage"/>
>-        <menuseparator id="messagePaneContext-sep-copy"/>
>-        <menuitem id="context-savelink"
>-                  label="&saveLinkCmd.label;"
>-                  accesskey="&saveLinkCmd.accesskey;"
>-                  oncommand="gContextMenu.saveLink();"/>
>-        <menuitem id="context-saveimage"
>-                  label="&saveImageCmd.label;"
>-                  accesskey="&saveImageCmd.accesskey;"
>-                  oncommand="gContextMenu.saveImage();"/>
>+    <menuseparator id="messagePaneContext-sep-copy"/>
>+    <menuitem id="context-savelink"
>+              label="&saveLinkCmd.label;"
>+              accesskey="&saveLinkCmd.accesskey;"
>+              oncommand="gContextMenu.saveLink();"/>
>+    <menuitem id="context-saveimage"
>+              label="&saveImageCmd.label;"
>+              accesskey="&saveImageCmd.accesskey;"
>+              oncommand="gContextMenu.saveImage();"/>
>         <menuitem id="context-bookmarklink"
>                   label="&bookmarkLinkCmd.label;"
>                   accesskey="&bookmarkLinkCmd.accesskey;"
>                   oncommand="initBMService();
>                              BookmarksUtils.addBookmark(gContextMenu.linkURL(),
>                                                         gContextMenu.linkText());"/>
>-
> </popup>
Again, only part of the job done here.
(In reply to comment #5)
> Don't you need to reindent the first two menuitems too?

I plan to do this separately in a next patch to deal with the "Identical code, but not at the same place" part of this bug.

> Again, only part of the job done here.

This was on purpose.
Is it acceptable ?
Attachment #286406 - Flags: superreview?(mscott)
Attachment #286406 - Flags: superreview+
Attachment #286406 - Flags: review?(mscott)
Attachment #286406 - Flags: review+
Keywords: checkin-needed
Target Milestone: --- → seamonkey2.0alpha
Attachment #286407 - Flags: superreview?(neil) → superreview+
Whiteboard: [c-n: Av1a-TB]
Comment on attachment 286406 [details] [diff] [review]
(Av1a-TB) whitespaces
[Checkin: comment 7]

{{
2007-11-04 06:16	bugzilla%standard8.plus.com 	mozilla/mail/base/content/mailWindowOverlay.xul 	1.227
}}
Attachment #286406 - Attachment description: (Av1a-TB) whitespaces → (Av1a-TB) whitespaces [Checkin: comment 7]
Keywords: checkin-needed
Whiteboard: [c-n: Av1a-TB]
Attachment #286407 - Flags: review?(iann_bugzilla) → review+
Keywords: checkin-needed
Whiteboard: [c-n: Bv1-SM]
Checked in:

Checking in mailnews/base/resources/content/mailWindowOverlay.xul;
/cvsroot/mozilla/mailnews/base/resources/content/mailWindowOverlay.xul,v  <--  mailWindowOverlay.xul
new revision: 1.339; previous revision: 1.338
Keywords: checkin-needed
Whiteboard: [c-n: Bv1-SM]
Attachment #286407 - Attachment description: (Bv1-SM) whitespaces → (Bv1-SM) whitespaces [Checkin: comment 8]
*some whitespace cleanups/synchronizations.
*a few line reorderings.
*some |;| additions.
Attachment #287403 - Flags: superreview?(mscott)
Attachment #287403 - Flags: review?(mscott)
Attached patch (Dv1-SM) whitespaces and reorder (obsolete) — Splinter Review
*some whitespace cleanups/synchronizations.
*a few line reorderings.
 *This changes the menuitem order.!.
*some |;| additions.
Attachment #287405 - Flags: superreview?(neil)
Attachment #287405 - Flags: review?(iann_bugzilla)
Comment on attachment 287405 [details] [diff] [review]
(Dv1-SM) whitespaces and reorder

>Index: mailWindowOverlay.xul
>===================================================================
>@@ -52,16 +52,16 @@
> <!DOCTYPE overlay [
> <!ENTITY % messengerDTD SYSTEM "chrome://messenger/locale/messenger.dtd" >
> %messengerDTD;
>-<!ENTITY % globalDTD SYSTEM "chrome://global/locale/global.dtd" >
>-%globalDTD;
>-<!ENTITY % msgViewPickerDTD SYSTEM "chrome://messenger/locale/msgViewPickerOverlay.dtd" >
>-%msgViewPickerDTD;
>+  <!ENTITY % globalDTD SYSTEM "chrome://global/locale/global.dtd">
>+  %globalDTD;
>+  <!ENTITY % msgViewPickerDTD SYSTEM "chrome://messenger/locale/msgViewPickerOverlay.dtd">
>+  %msgViewPickerDTD;
> <!ENTITY % msgHdrViewPopupDTD SYSTEM "chrome://messenger/locale/msgHdrViewPopup.dtd" >
> %msgHdrViewPopupDTD;
> <!ENTITY % contentAreaCommandsDTD SYSTEM "chrome://communicator/locale/contentAreaCommands.dtd" >
> %contentAreaCommandsDTD;
>-<!ENTITY % brandDTD SYSTEM "chrome://branding/locale/brand.dtd" >
>-%brandDTD;
>+  <!ENTITY % brandDTD SYSTEM "chrome://branding/locale/brand.dtd">
>+  %brandDTD;
> ]>
Why only indent some of the above?

>@@ -118,9 +118,7 @@
> 
>     <command id="cmd_emptyTrash" oncommand="goDoCommand('cmd_emptyTrash')" disabled="true"/>
>     <command id="cmd_compactFolder" oncommand="goDoCommand('cmd_compactFolder')" disabled="true"/>
>-    <command id="cmd_synchronizeOffline" oncommand="goDoCommand('cmd_synchronizeOffline')" disabled="true"/>
>     <commandset id="mailDownloadCommands"/>
>-    <command id="cmd_settingsOffline" oncommand="goDoCommand('cmd_settingsOffline')" disabled="true"/>
> 
>     <command id="cmd_printSetup" oncommand="goDoCommand('cmd_printSetup')" disabled="true"/>
>     <command id="cmd_print" oncommand="goDoCommand('cmd_print')" disabled="true"/>
>@@ -130,6 +128,8 @@
>     <command id="cmd_getNextNMessages" oncommand="goDoCommand('cmd_getNextNMessages')" disabled="true"/>
>     <command id="cmd_renameFolder" oncommand="goDoCommand('cmd_renameFolder')" />
>     <command id="cmd_sendUnsentMsgs" oncommand="goDoCommand('cmd_sendUnsentMsgs')" />
>+    <command id="cmd_synchronizeOffline" oncommand="goDoCommand('cmd_synchronizeOffline');" disabled="true"/>
>+    <command id="cmd_settingsOffline" oncommand="goDoCommand('cmd_settingsOffline');" disabled="true"/>
> </commandset>
> 
> <commandset id="mailCommands">
>@@ -228,7 +228,7 @@
>   <command id="cmd_previousFlaggedMsg" oncommand="goDoCommand('cmd_previousFlaggedMsg')" disabled="true"/>
>   <command id="cmd_goStartPage" oncommand="goDoCommand('cmd_goStartPage');"/>
>   <command id="cmd_goBack" oncommand="goDoCommand('cmd_goBack')" disabled="true"/>
>-  <command id="cmd_goForward" oncommand="goDoCommand('cmd_goForward')" disabled="true"/>
>+  <command id="cmd_goForward" oncommand="goDoCommand('cmd_goForward');" disabled="true"/>
> </commandset>
Why the above change and not to other lines without a |;|?

> 
> <commandset id="mailMessageMenuItems"
>@@ -354,7 +354,7 @@
>   <key id="key_previousMsg" key="&previousMsgCmd.key;"               oncommand="goDoCommand('cmd_previousMsg')"/>
>   <key id="key_previousUnreadMsg" key="&previousUnreadMsgCmd.key;"   oncommand="goDoCommand('cmd_previousUnreadMsg')"/>
>   <key id="key_goBack" key="&goBackCmd.commandKey;"                  oncommand="goDoCommand('cmd_goBack')"/>
>-  <key id="key_goForward" key="&goForwardCmd.commandKey;"            oncommand="goDoCommand('cmd_goForward')"/>
>+  <key id="key_goForward" key="&goForwardCmd.commandKey;"            oncommand="goDoCommand('cmd_goForward');"/>
Again why this change and not other lines too?

Same question again for indentation of menuitems, why some and not others?
Attachment #287405 - Flags: review?(iann_bugzilla) → review-
(In reply to comment #11)
> (From update of attachment 287405 [details] [diff] [review])
> Why only indent some of the above?
> Why the above change and not to other lines without a |;|?
> Again why this change and not other lines too?
> Same question again for indentation of menuitems, why some and not others?

(Same kind of answer as comment 6:)
I'm doing it incrementally:
I'm modifying only the lines which I need to touch in either SM or TB in order to synchronize them;
the unmodified lines are either identical or "not synchronised yet";
I plan to first fix the later, then do an additional format patch as needed in the end.

Is this intermediate state acceptable ?
I don't think IanN got to read comment 12...
Comment on attachment 287405 [details] [diff] [review]
(Dv1-SM) whitespaces and reorder

(In reply to comment #12)
> (In reply to comment #11)
> > (From update of attachment 287405 [details] [diff] [review] [details])
> > Why only indent some of the above?
> > Why the above change and not to other lines without a |;|?
> > Again why this change and not other lines too?
> > Same question again for indentation of menuitems, why some and not others?
> 
> (Same kind of answer as comment 6:)
> I'm doing it incrementally:
> I'm modifying only the lines which I need to touch in either SM or TB in order
> to synchronize them;
> the unmodified lines are either identical or "not synchronised yet";
> I plan to first fix the later, then do an additional format patch as needed in
> the end.
> 
> Is this intermediate state acceptable ?
> 
I would still say all the <!ENTITY> changes should be done as part of this patch, the other changes can wait to a later patch.

r=me with the above fixed.
Attachment #287405 - Flags: review- → review+
Dv1-SM, with comment 14 update.
Attachment #287405 - Attachment is obsolete: true
Attachment #287776 - Flags: superreview?(neil)
Attachment #287405 - Flags: superreview?(neil)
Attachment #287776 - Flags: superreview?(neil) → superreview+
Keywords: checkin-needed
Whiteboard: [c-n: Dv1a-SM]
Dv1a-SM:

Checking in mailnews/base/resources/content/mailWindowOverlay.xul;
/cvsroot/mozilla/mailnews/base/resources/content/mailWindowOverlay.xul,v  <--  mailWindowOverlay.xul
new revision: 1.340; previous revision: 1.339
done
Keywords: checkin-needed
Whiteboard: [c-n: Dv1a-SM]
Attachment #287776 - Attachment description: (Dv1a-SM) whitespaces and reorder → (Dv1a-SM) whitespaces and reorder [Checkin: Comment 16]
Comment on attachment 287403 [details] [diff] [review]
(Cv1a-TB) whitespaces and reorder

Phil, would you have more time than Scott to process this simple request ?
Attachment #287403 - Flags: superreview?(mscott) → review?(philringnalda)
Comment on attachment 287403 [details] [diff] [review]
(Cv1a-TB) whitespaces and reorder

Sorry, but no. One patch which does every bit of content-free changing that you want to do, please. Whatever your unexplained reason for wanting to do it in an infinite series of patches is, it doesn't trump the fact that having multiple revisions of whitespace and shuffling, especially if they land with real changes in between them, makes using CVS blame a miserable annoyance.
Attachment #287403 - Flags: review?(philringnalda)
Attachment #287403 - Flags: review?(mscott)
Attachment #287403 - Flags: review-
While working on bug 68174, I noticed bug 350661 comment 5.
Should I port that TB feature to SM !?
And same question about bug 350543. :->
(In reply to comment #19)
> While working on bug 68174, I noticed bug 350661 comment 5.
> Should I port that TB feature to SM !?
(In reply to comment #20)
> And same question about bug 350543. :->

That'd be awesome! :)
(In reply to comment #21)
> (In reply to comment #19)
> > While working on bug 68174, I noticed bug 350661 comment 5.
> > Should I port that TB feature to SM !?
> (In reply to comment #20)
> > And same question about bug 350543. :->
> 
> That'd be awesome! :)
> 
Definitely, are you intending on working on them in the above mentioned bugs, this one or creating separate one(s)? If the latter, not them here and the bugs mentioned.
Depends on: 416669
Depends on: 427972
(In reply to comment #22)
> If the latter, not them here and the bugs mentioned.

I filed bug 416669 and bug 427972.
This bug is open but targeted for seamonkey2.0a1, which has been released a long time ago. Please set the target milestone to an appropriate value, "---" if it has no specific target.
Target Milestone: seamonkey2.0a1 → ---
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: