664105 - TEST-UNEXPECTED-FAIL | test_URIs.js | false == true for Thunderbird when running mailto: test

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Mark Banner (:standard8)]

With the patch on bug 658666, we're now running xpcshell-tests on mozilla-central again. test_URIs.js is now failing, because of a test added by bug 660066:

TEST-INFO | /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/netwerk/test/unit/test_URIs.js | [do_test_uri_basic : 257] testing mailto:[e-mail] equals a clone of itself

TEST-INFO | /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/netwerk/test/unit/test_URIs.js | [do_check_uri_eq : 208] (uri equals check: 'mailto:[e-mail]' == 'mailto:[e-mail]')

TEST-PASS | /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/netwerk/test/unit/test_URIs.js | [do_check_uri_eq : 209] true == true

TEST-INFO | /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/netwerk/test/unit/test_URIs.js | [do_check_uri_eq : 210] (uri equals check: 'mailto:[e-mail]' == 'mailto:[e-mail]')

TEST-UNEXPECTED-FAIL | /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/netwerk/test/unit/test_URIs.js | false == true - See following stack:
JS frame :: /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/head.js :: do_throw :: line 445
JS frame :: /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/head.js :: _do_check_eq :: line 539
JS frame :: /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/head.js :: do_check_eq :: line 560
JS frame :: /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/head.js :: do_check_true :: line 574
JS frame :: /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/netwerk/test/unit/test_URIs.js :: do_check_uri_eq :: line 211
JS frame :: /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/netwerk/test/unit/test_URIs.js :: do_test_uri_basic :: line 258
JS frame :: /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/netwerk/test/unit/test_URIs.js :: <TOP_LEVEL> :: line 442
JS frame :: /buildbot/comm-central-linux-opt-unittest-xpcshell/build/xpcshell/tests/netwerk/test/unit/test_URIs.js :: run_test :: line 440

I think this is saying that URI == URI.clone(), but URI.clone() != URI for mailto URIs.

This would seem to be an issue with the code here:

http://hg.mozilla.org/comm-central/diff/e4f4569d451a/mailnews/compose/src/nsSmtpUrl.cpp

Comment 1

[[e-mail]]

Attachment: Proposed patch

I originally wrote this for bug 658877 as I noticed that some of the methods didn't work as advertised.

Comment 2

[David :Bienvenu]

If other is null, isn't this eventually going to return NS_ERROR_NULL_POINTER? In other words, couldn't we do NS_ENSURE_ARG_POINTER(other)? Or is there a reason to make the base url do it?

+  if (other)
+    return other->Equals(m_baseURL, _retval);
+
+  return m_baseURL->Equals(other, _retval);

Comment 3

[[e-mail]]

(In reply to comment #2)
> If other is null, isn't this eventually going to return
> NS_ERROR_NULL_POINTER? In other words, couldn't we do
> NS_ENSURE_ARG_POINTER(other)? Or is there a reason to make the base url do
> it?
1. It's possible that Equals might change its behaviour on a null parameter, and this lets us pick up that change for free.
2. You didn't complain in bug 546900 ;-)

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 539174 [details] [diff] [review]
Proposed patch

David reviewed this, and I believe it landed anyway.

Comment 5

[Mark Banner (:standard8)]

This landed here ages ago:

http://hg.mozilla.org/comm-central/rev/a9dbef5819f8