Closed Bug 22957 Opened 20 years ago Closed 11 years ago

move openWindow() out of nsNNTPNewsgroupList.cpp and nsMsgComposeService.cpp


(MailNews Core :: Backend, defect, P2)



(Not tracked)

Thunderbird 3.0b2


(Reporter: sspitzer, Assigned: standard8)



(1 file)

right now, both nsNNTPNewsgroupList.cpp and nsMsgComposeService have a static
function openWindow().  news uses it to launch the download headers dialog and
compose uses it to open the compose window.

to make it work, both compose and news are linking against the javascript
library, to get JS_PushArguments() and JS_PopArguments

the right thing to do would be to move this code into the nsCommonDialog service
code.   we want the ability to launch any given xul url as a dialog or as a window.

the second step will be to pass the arguments not as a PRUnichar * (like
"arg1=val1,arg2=val2", but as nsIDialogParamBlock or something like that.
see DoDialog()
Target Milestone: M13
marking m13, accepting.
Target Milestone: M13 → M14
moving this to m14.
QA Contact: lchiang → ppandit
Priority: P3 → P2
No user-visible downside is described here, so pushing out to M15.
Target Milestone: M14 → M15
I'm nsIDialogParamBlock in nsNNTPNewsgroupList.cpp (and eventually I'll switch
to nsINewsDownloadDialogArgs)

now that I understand what I'm doing, I can fix openWindow in
nsMsgComposeService and pass arguments without having to parse them from js.
moving to m16.
Target Milestone: M15 → M16
Triage to M17.  Please add beta2 keyword if this must make beta2.  Please let me 
know if this must be done by M16 feature freeze.
Target Milestone: M16 → M17
moving to future milestone.
Target Milestone: M17 → Future
QA Contact: ppandit → stephend
Product: MailNews → Core
sorry for the spam.  making bugzilla reflect reality as I'm not working on these bugs.  filter on FOOBARCHEESE to remove these in bulk.
Assignee: sspitzer → nobody
Filter on "Nobody_NScomTLD_20080620"
QA Contact: stephend → backend
Product: Core → MailNews Core
Although we still have OpenWindow in nsMsgComposeService.cpp, IMHO that's the right place for it.

Additionally, it doesn't use any JS functions. So I think we can stop linking against the js libs. This works for me on Mac.

If this doesn't work on other platforms, then we'll just close this bug. Otherwise we'll do this patch, then close this bug.
Assignee: nobody → bugzilla
Attachment #350760 - Flags: superreview?(neil)
Attachment #350760 - Flags: review?(neil)
(In reply to comment #11)
> Created an attachment (id=350760) [details]
> Don't link against JS libs
> Additionally, it doesn't use any JS functions. So I think we can stop linking
> against the js libs. This works for me on Mac.

Also works on Linux.
Attachment #350760 - Flags: superreview?(neil)
Attachment #350760 - Flags: superreview+
Attachment #350760 - Flags: review?(neil)
Attachment #350760 - Flags: review+
Comment on attachment 350760 [details] [diff] [review]
Don't link against JS libs

> 		$(LIBS_DIR) \
>-		$(MOZ_JS_LIBS) \
> 		$(NULL)
>diff --git a/mailnews/news/build/ b/mailnews/news/build/
>--- a/mailnews/news/build/
>+++ b/mailnews/news/build/
>@@ -85,9 +85,8 @@ EXTRA_DSO_LDOPTS = \
> 		-L$(DIST)/bin \
> 		-L$(DIST)/lib \
Not part of this bug but this should be $(LIBS_DIR) too, no?
Checked in
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: Future → Thunderbird 3.0b2
You need to log in before you can comment on or make changes to this bug.