Last Comment Bug 762393 - merge junkLog.* files in /mailnews and /mail
: merge junkLog.* files in /mailnews and /mail
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Filters (show other bugs)
: Trunk
: All All
: -- minor (vote)
: Thunderbird 16.0
Assigned To: :aceman
:
:
Mentors:
Depends on: 760126
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-07 00:21 PDT by :aceman
Modified: 2012-06-18 15:51 PDT (History)
3 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (11.28 KB, patch)
2012-06-07 14:56 PDT, :aceman
iann_bugzilla: review+
Details | Diff | Splinter Review
patch v2 (13.26 KB, patch)
2012-06-10 04:30 PDT, :aceman
mconley: review+
Details | Diff | Splinter Review

Description :aceman 2012-06-07 00:21:01 PDT
The junkLog.* files in /mailnews/base/content/ and /mail/components/preferences/ look identical. The only difference I see is a string ID that was changed for bug 323159, that was done only in the TB version. But maybe Seamonkey would also like that change.

http://mxr.mozilla.org/comm-central/search?string=junklog.xul&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central

http://mxr.mozilla.org/comm-central/search?string=junklog.js&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central

From my understanding Seamonkey uses the /mailnews files and Tb uses the /mail files. So I'd try to merge the files and make both TB and SM use the /mailnews version (that will be updated for the string ID change). I assume that should be just some jar.mn touches and using "chrome://messenger/content/junkLog.*" as src in the files referencing those 2 files.

(But wait until bug 760126 lands.)
Comment 1 Ian Neal 2012-06-07 12:56:12 PDT
(In reply to :aceman from comment #0)
> The junkLog.* files in /mailnews/base/content/ and
> /mail/components/preferences/ look identical. The only difference I see is a
> string ID that was changed for bug 323159, that was done only in the TB
> version. But maybe Seamonkey would also like that change.
I see no problem with SeaMonkey having that string change.
Comment 2 :aceman 2012-06-07 14:23:48 PDT
Thanks, I'll go for it.

Ian, I usually change the status to ASSIGNED (on bugs where I am the assignee) only after I produce any patch :) It helps me keep some order in the bugs.
Comment 3 :aceman 2012-06-07 14:56:11 PDT
Created attachment 631163 [details] [diff] [review]
patch

Ian, please see if Seamonkey is unaffected and only gets the string change.

I can confirm that at least on TB12 both /mailnews and /mail copies of junkLog.* files are shipped in omni.ja. So this should reduce the size of release build of TB by about 2KB (before compression)! :-P
Comment 4 Ian Neal 2012-06-09 17:20:15 PDT
Comment on attachment 631163 [details] [diff] [review]
patch

>+++ b/mail/locales/jar.mn
>@@ -143,17 +143,17 @@
>   locale/@AB_CD@/messenger/messengercompose/mailComposeEditorOverlay.dtd (%chrome/messenger/messengercompose/mailComposeEditorOverlay.dtd)
>   locale/@AB_CD@/messenger/preferences/preferences.dtd                  (%chrome/messenger/preferences/preferences.dtd)
>   locale/@AB_CD@/messenger/preferences/general.dtd                      (%chrome/messenger/preferences/general.dtd)
>   locale/@AB_CD@/messenger/preferences/display.dtd                      (%chrome/messenger/preferences/display.dtd)
>   locale/@AB_CD@/messenger/preferences/chat.dtd                         (%chrome/messenger/preferences/chat.dtd)
>   locale/@AB_CD@/messenger/preferences/compose.dtd                      (%chrome/messenger/preferences/compose.dtd)
>   locale/@AB_CD@/messenger/preferences/sendoptions.dtd                  (%chrome/messenger/preferences/sendoptions.dtd)
>   locale/@AB_CD@/messenger/preferences/security.dtd                     (%chrome/messenger/preferences/security.dtd)
>-  locale/@AB_CD@/messenger/preferences/junkLog.dtd                      (%chrome/messenger/preferences/junkLog.dtd)
>+  locale/@AB_CD@/messenger/junkLog.dtd                                  (%chrome/messenger/junkLog.dtd)
You might want to put this earlier in the file.

>+++ b/mailnews/base/content/junkLog.xul
>     <hbox>
>+      <label value="&adaptiveJunkLogInfo.label;"/>
>       <spacer flex="1"/>
>+      <button label="&clearLog.label;" accesskey="&clearLog.accesskey;"
>+              oncommand="clearLog();"/>
Nit: one attribute per line.
>     </hbox>
>     <vbox flex="1">
>       <spacer height="10px"/>
>       <hbox flex="1">
>         <spacer width="10px"/>
>+        <browser id="logView" disablehistory="true" disablesecurity="true"
>+                 src="about:blank" autofind="false" flex="1"/>
Nit: one attribute per line.

r=me with those fixed/addressed
Comment 5 :aceman 2012-06-10 04:30:59 PDT
Created attachment 631733 [details] [diff] [review]
patch v2

Thanks Ian.
There should be no UI change in TB (only string change in SM) so no ui-r requested.
Comment 6 Mike Conley (:mconley) 2012-06-18 07:10:55 PDT
Comment on attachment 631733 [details] [diff] [review]
patch v2

Review of attachment 631733 [details] [diff] [review]:
-----------------------------------------------------------------

Looks OK to me. Thanks for the patch!
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-06-18 15:51:53 PDT
https://hg.mozilla.org/comm-central/rev/47380a5f11f4

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