611029 - Move abSelectAddressesDialog.xul to /suite/mailnews

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: General, defect)

Description

[Jens Hatlak (:InvisibleSmiley)]

/mailnews/addrbook/content/abSelectAddressesDialog.xul is SM-specific, should be moved to /suite/mailnews. Same for abSelectAddressesDialog.js and probably also addressbook-panel.xul.

Comment 1

[Edmund Wong (:ewong)]

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.

Comment 2

[Jens Hatlak (:InvisibleSmiley)]

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

Comment 3

[Karsten Düsterloh]

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

Comment 4

[Edmund Wong (:ewong)]

Attachment: Moved abSelectAddressesDialog.(xul|js) and addressbook-panel.xul to suite/

Comment 5

[Jens Hatlak (:InvisibleSmiley)]

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 6

[Karsten Düsterloh]

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.

Comment 7

[Edmund Wong (:ewong)]

Attachment: Moved abSelectAddressesDialog.(xul|js) and addressbook-panel.(xul|js) to suite/ (v2)

Comment 8

[Karsten Düsterloh]

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.

Comment 9

[Karsten Düsterloh]

Landed as <http://hg.mozilla.org/comm-central/rev/d1e3e4add0fe>.