* Whitespaces already removed from mail/ file. * Identical code, but not at the same place. * Then, investigate code which is different.

Comment on attachment 286403 [details] [diff] [review] (Av1-TB) whitespaces (Wrong patch, sorry.)

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 ?

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 }}

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

*some whitespace cleanups/synchronizations. *a few line reorderings. *some |;| additions.

*some whitespace cleanups/synchronizations. *a few line reorderings. *This changes the menuitem order.!. *some |;| additions.

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?

(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.

Dv1-SM, with comment 14 update.

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

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 ?

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.

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.

(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.