Move abSelectAddressesDialog.xul to /suite/mailnews

RESOLVED FIXED in seamonkey2.1b2

Status

SeaMonkey
MailNews: General
--
minor
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: InvisibleSmiley, Assigned: ewong)

Tracking

Trunk
seamonkey2.1b2
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
/mailnews/addrbook/content/abSelectAddressesDialog.xul is SM-specific, should be moved to /suite/mailnews. Same for abSelectAddressesDialog.js and probably also addressbook-panel.xul.
Flags: in-testsuite-
Whiteboard: [good first bug]
(Assignee)

Updated

7 years ago
Assignee: nobody → ewong
Status: NEW → ASSIGNED
(Assignee)

Comment 1

7 years ago
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.
(Reporter)

Comment 2

7 years ago
(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

7 years ago
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.)
(Assignee)

Comment 4

7 years ago
Created attachment 492995 [details] [diff] [review]
Moved abSelectAddressesDialog.(xul|js) and addressbook-panel.xul to suite/
Attachment #492995 - Flags: review?
(Assignee)

Updated

7 years ago
Attachment #492995 - Flags: review? → review?(mnyromyr)
(Reporter)

Comment 5

7 years ago
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

7 years ago
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-
(Assignee)

Comment 7

7 years ago
Created attachment 493263 [details] [diff] [review]
Moved abSelectAddressesDialog.(xul|js) and addressbook-panel.(xul|js) to suite/ (v2)
Attachment #492995 - Attachment is obsolete: true
Attachment #493263 - Flags: review?(mnyromyr)

Comment 8

7 years ago
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)
Attachment #493263 - Flags: review?(mnyromyr)
Attachment #493263 - Flags: review+
Attachment #493263 - Flags: superreview?(bugzilla) → superreview+

Comment 9

7 years ago
Landed as <http://hg.mozilla.org/comm-central/rev/d1e3e4add0fe>.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Reporter)

Updated

7 years ago
Target Milestone: --- → seamonkey2.1b2
You need to log in before you can comment on or make changes to this bug.