658877 - fix nsIURI build bustage caused by fix for bug 308590

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Comment 1

[David :Bienvenu]

will try to work on this this afternoon

Comment 2

[David :Bienvenu]

Attachment: patch to get us linking

this just defers to the base url (except for Equals and EqualsExceptRef, which probably could do the same)

I'm going to land this, and see what the test results are, because I have to run out in a minute

Comment 3

[David :Bienvenu]

http://hg.mozilla.org/comm-central/rev/b656d0bd1cfc pushed

Comment 4

[Ian Neal]

Attachment: nsSmtpUrl fix

This might help make more greenness (if it is correct).

Comment 5

[Ian Neal]

Hmmm, not quite, I get:
comm-central/mailnews/compose/src/nsSmtpService.cpp: In function ‘nsresult NS_MsgBuildSmtpUrl(nsIFile*, nsISmtpServer*, const char*, nsIMsgIdentity*, nsIUrlListener*, nsIMsgStatusFeedback*, nsIInterfaceRequestor*, nsIURI**, PRBool)’:
comm-central/mailnews/compose/src/nsSmtpService.cpp:181:54: warning: enumeral mismatch in conditional expression: ‘nsISmtpUrl::<anonymous enum>’ vs ‘nsISmtpUrl::<anonymous enum>’
comm-central/mailnews/compose/src/nsSmtpService.cpp: In static member function ‘static PRBool nsSmtpService::findServerByHostname(nsISmtpServer*, void*)’:
comm-central/mailnews/compose/src/nsSmtpService.cpp:727:79: warning: suggest parentheses around ‘&&’ within ‘||’

Comment 6

[Ian Neal]

Attachment: Fix kB and sync patch (with semi-colons)

Ok, this time with the missing semi-colons in place

Comment 7

[David :Bienvenu]

Ian, I don't think that last patch is the right one for this bug (at least, for me to review)

Comment 8

[David :Bienvenu]

sorry, I forgot to push one directory - compose change pushed.

Comment 9

[Ian Neal]

Attachment: The correct patch with semi-colons

Oops, too many windows and bugs being worked on. This should be the correct patch.
Tested locally.

Comment 10

[[e-mail]]

For mailto: and addbook: URLs I think we can just ignore or throw on ref operations. But unfortunately I don't think we can do this for mail/news URLs because of internal anchors. (CloneIgnoringRef is particularly tricky.)

Comment 11

[Ludovic Hirlimann [:Usul]]

(In reply to comment #10)
> For mailto: and addbook: URLs I think we can just ignore or throw on ref
> operations. But unfortunately I don't think we can do this for mail/news
> URLs because of internal anchors. (CloneIgnoringRef is particularly tricky.)

Joshua thoughts ?

Comment 12

[Joshua Cranmer [:jcranmer]]

(In reply to comment #10)
> For mailto: and addbook: URLs I think we can just ignore or throw on ref
> operations. But unfortunately I don't think we can do this for mail/news
> URLs because of internal anchors. (CloneIgnoringRef is particularly tricky.)

For news URLs, the only ones that have the anchor are the news-message: URIs, which shouldn't be a problem since we only create and consume such nsNntpUrl objects internally, and don't release them outside where someone might want to start calling Clone. That should be true for all *-message URIs; at a pinch, CloneIgnoringRef could simply treat those as not being ref portions of the URI.

Or, barring that, we might be able to s/#/;/ in all *-message URI stuff. I recall doing archaeology on thew *-message stuff, and, IIRC, the primary reason for using `#' was something related with RDF which has long since ceased to be the case. s/#/;/ might also make it play better with URIs... running around with a URI where the ref is meaningful is just asking for trouble :-) .

Comment 13

[David :Bienvenu]

Comment on attachment 534331 [details] [diff] [review]
The correct patch with semi-colons

don't think we need this

Comment 14

[David :Bienvenu]

I think we can mark this fixed now and deal with remaining issues in other bugs (which I think we've already been doing)