Closed Bug 466880 Opened 12 years ago Closed 12 years ago

move Thunderbird fork of mailWidgets.xml to mail/


(Thunderbird :: Mail Window Front End, defect)

Not set


(Not tracked)



(Reporter: kairo, Assigned: kairo)




(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/ b/mail/base/

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

Attachment #350391 - Flags: review?(david.ascher) → review+
Attachment #350391 - Flags: review?(bugzilla)
Attachment #350391 - Flags: review?(bugzilla) → review+
Pushed as
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.