804690 - security/manager/ssl/tests/unit/test_bug627234.js is perma-orange on Thunderbird's comm-*

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

Not sure if this is the right component for this one, but Thunderbird has been perma-oranged by a test in security/manager/ssl/tests/ that assumes that private browsing is a registered component.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1

This patch checks for the private browsing service, and if it isn't registered, skips the test.

Comment 2

[Mark Banner (:standard8)]

This was caused by bug 627234 and is now on aurora and beta as well as the test has also landed on those branches.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 674305 [details] [diff] [review]
Patch v1

Maybe David Keeler has some cycles?

Comment 4

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 674305 [details] [diff] [review]
Patch v1

Review of attachment 674305 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Sorry this happened again. Hopefully I'll remember next time.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Thanks for the review - landed on inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/6ddc79908e64

Comment 6

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 674305 [details] [diff] [review]
Patch v1

Review of attachment 674305 [details] [diff] [review]:
-----------------------------------------------------------------

I get nervous about these "if we can't get the private browsing service then succeed without running the test" tests. It seems like there should be some test component with a function that returns true if private browsing is supposed to work, and false if private browsing isn't supposed to work--perhaps a "disable private browsing" pref that Thunderbird can set and Firefox can leave unset. Something for the future, I guess.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6ddc79908e64

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 674305 [details] [diff] [review]
Patch v1

Bug 627234 landed on Aurora and Beta as well, and has busted Thunderbird's tests there too.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 627234
User impact if declined: None.
Testing completed (on m-c, etc.): mozilla-central
Risk to taking this patch (and alternatives if risky): Very little, since it's only test related.
String or UUID changes made by this patch: None.

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 674305 [details] [diff] [review]
Patch v1

I've been told tha test only fixes can land with a=testonly (which is how the original test landed), so we don't need explicit approval here.

Comment 10

[Mark Banner (:standard8)]

Checked in with a=testonly:

https://hg.mozilla.org/releases/mozilla-aurora/rev/1bc9ccbdd7db
https://hg.mozilla.org/releases/mozilla-beta/rev/57fcc3cd329a