Closed Bug 611029 Opened 9 years ago Closed 9 years ago
Select Addresses Dialog .xul to /suite/mailnews
/mailnews/addrbook/content/abSelectAddressesDialog.xul is SM-specific, should be moved to /suite/mailnews. Same for abSelectAddressesDialog.js and probably also addressbook-panel.xul.
Whiteboard: [good first bug]
From my preliminary findings, I'm not sure about moving addressbook-panel.xul since it is referenced in http://mxr.mozilla.org/comm-central/source/mailnews/addrbook/content/abResultsPane.js#202.
(In reply to comment #1) > From my preliminary findings, I'm not sure about moving addressbook-panel.xul > since it is referenced in > http://mxr.mozilla.org/comm-central/source/mailnews/addrbook/content/abResultsPane.js#202. And that file includes and uses abSelectAddressesDialog.js: - DirPaneSelectionChangeMenulist() - onEnterInSearchBar() What confuses me is that the file also includes abSelectAddressesDialog.dtd which is only present under suite/, so I wonder whether TB only calls GetSelectedAbCards (in abResultsPane.js) in contexts where no sidebar-box element is present (which would render that code part dead for TB). I guess you'll have to check the four TB occurrences: http://mxr.mozilla.org/comm-central/ident?i=GetSelectedAbCards Or ask one of Mnyromyr, Standard8, bienvenu, etc.
abSelectAddressesDialog.js is only used by abSelectAddressesDialog.xul and addressbook-panel.xul, and those two are only used by SM, hence all three can move into /suite. TB's addressing sidebar in the mail editor is abContactsPanel.xul, which loads abResultsPane.js *on the inside*, hence the check for "sidebar-box" will always fail. Other uses (addressbook main window, address search window) don't even have a "sidebar-box" either. (And even if, gCurFrame is only defined in SM's sidebarOverlay.js.) In fact, the code block in question doesn't work in SM as well at the moment, because we don't have the sidebar back in the Messenger yet... (No need to touch that here, it's out of scope for this bug.)
Attachment #492995 - Flags: review? → review?(mnyromyr)
Comment on attachment 492995 [details] [diff] [review] Moved abSelectAddressesDialog.(xul|js) and addressbook-panel.xul to suite/ I think addressbook-panel.js can be moved, too. FYI: Once you have r+ from Karsten, you also need to get a review (sr, in this case) from a TB dev (usually Standard8 or bienvenu) since /mailnews is under their control.
Comment on attachment 492995 [details] [diff] [review] Moved abSelectAddressesDialog.(xul|js) and addressbook-panel.xul to suite/ >diff --git a/mailnews/mailnews.js b/mailnews/mailnews.js >-// see abCommon.js and abSelectAddressesDialog.js >+// see abCommon.js > pref("mailnews.ui.select_addresses_results.version", 1); Please don't change this. The pref is only used by (or rather: known to) SM and should be moved by bug 496429 anyway. (In reply to comment #5) > I think addressbook-panel.js can be moved, too. Oh, yes, of course. Please add this as well.
Attachment #492995 - Flags: review?(mnyromyr) → review-
Comment on attachment 493263 [details] [diff] [review] Moved abSelectAddressesDialog.(xul|js) and addressbook-panel.(xul|js) to suite/ (v2) Yep, that's it. Mark, please see comment #3 for reasons why TB isn't affected.
Attachment #493263 - Flags: superreview?(bugzilla) → superreview+
Landed as <http://hg.mozilla.org/comm-central/rev/d1e3e4add0fe>.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b2
You need to log in before you can comment on or make changes to this bug.