Closed Bug 466880 Opened 16 years ago Closed 16 years ago

move Thunderbird fork of mailWidgets.xml to mail/

Categories

(Thunderbird :: Mail Window Front End, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: kairo, Assigned: kairo)

References

Details

Attachments

(1 file)

Now that SeaMonkey has forked mailWidgets.xml into suite/ we should move the Thunderbird copy to mail/ and remove the non-Thunderbird branches of ifdefs, etc.

I'll look into this tomorrow, no need for otherwise busy Thunderbird people to work on it ;-)
This patch moves the Thunderbird mailWidgets.xml to mail/ and removes the ifdefs as well as the JS switch for folder pane (with some bonus simplification).
Attachment #350391 - Flags: review?
Attachment #350391 - Flags: review? → review?(david.ascher)
Comment on attachment 350391 [details] [diff] [review]
move Thunderbird file to mail/ and remove ifdefs

I don't have the complete background on the following, but it looks like a good change.  That first if statement looks horrible, i'm glad to see it gone.

>-          var msgFolder;
>-          // Allow for differences between Thunderbird and SeaMonkey folder pane
>-          if ("gFolderTreeView" in window) {
>-            msgFolder = gFolderTreeView.getFolderAtCoords(event.clientX,
>-                                                          event.clientY);
>-            if (!msgFolder)
>-              return false;
>-          } else {
>-            var folderTree = GetFolderTree();
>-            var row = folderTree.treeBoxObject.getRowAt(event.clientX,
>-                                                        event.clientY);
>-            if (row == -1)
>-              return false;
>-            msgFolder = GetFolderResource(folderTree, row).QueryInterface(Components.interfaces.nsIMsgFolder);
>-          }
>+          var msgFolder = gFolderTreeView.getFolderAtCoords(event.clientX,
>+                                                            event.clientY);
>           if (!msgFolder || msgFolder.isServer)
>             return false;


Only one nit:

>diff --git a/mail/base/jar.mn b/mail/base/jar.mn

>-*   content/messenger/mailWidgets.xml               (/mailnews/base/resources/content/mailWidgets.xml)
>+    content/messenger/mailWidgets.xml               (content/mailWidgets.xml)

I think it'd be good to have the new line nearer to the other forked files, not in the middle of the /mailnews files.

I'm fine w/ the patch, but I don't have formal review privileges, and so it feels like you should get a formal review from someone else.

--david
Attachment #350391 - Flags: review?(david.ascher) → review+
Attachment #350391 - Flags: review?(bugzilla)
Attachment #350391 - Flags: review?(bugzilla) → review+
Pushed as http://hg.mozilla.org/comm-central/rev/b5c32640e85b
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.