989232 - [beta] Failure in testSecurity/testSafeBrowsingNotificationBar.js since bug 985623 has been backported

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Daniel Gherasim]

This test started failing on Beta across all platforms
http://mozmill-release.blargon7.com/#/remote/failure?app=All&branch=All&platform=All&from=2014-03-27&test=%2FtestSecurity%2FtestSafeBrowsingNotificationBar.js&func=testNotificationBar

Looks like it was introduced recently on beta 

Here is the regression range:
https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=e5d922ae5641&tochange=363950e031c0

From what we've seen in bug 985623, some changes we're not backported to beta.
Mainly the changes to libpref: https://hg.mozilla.org/releases/mozilla-aurora/diff/e1282ac83e49/modules/libpref/src/init/all.js are missing from Beta.

Monica could you please check if this was intended or maybe it was missed while resolving conflict?

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As it can be seen in the report referenced above, we actually are loading: URI=http://www.itisatrap.org/firefox/its-a-trap.html. Not sure why that happens given that bug 971240 hasn't been backported to beta yet.

Comment 2

[Daniel Gherasim]

Manually tested this:

So accessing http://www.mozilla.org/firefox/its-a-trap.html with the last build that work ( clean profile ) shows the Reported Web Forgery Page.

Accessing the same link on the first beta build (clean prof) on which the test fails redirects to http://www.itisatrap.org/firefox/its-a-trap.html and shows no Reported Web Page.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

So that means we miss the backport for bug 971240 on beta, or or shouldn't have backported bug 985623 to beta. This is a weird situation where we need feedback from everyone involved.

Comment 4

[Gian-Carlo Pascutto [:gcp]]

I doubt lack of bug 971240 is the issue, given that it updates the DB and the tests simultaneously. Given that bug 985623 had to be specifically changed for beta, that looks the most likely.

Comment 5

[Gian-Carlo Pascutto [:gcp]]

>Mainly the changes to libpref: https://hg.mozilla.org/releases/mozilla-aurora/diff/e1282ac83e49/modules/libpref/src/init/all.js are missing from Beta.

That's exactly right, those changes *are* needed on beta as well.

Comment 6

[[:mmc] Monica Chew (no longer reading bugmail)]

Yes, that's right. I had merge conflicts from uplifting  bug 985623 and missed the pref change. Patch coming.

Comment 7

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Fix failure in testSecurity/testSafeBrowsingNotificationBar.js (

Comment 8

[[:mmc] Monica Chew (no longer reading bugmail)]

I can't set the approval-beta flag for some reason. Sylvestre, this change is needed for beta only.

Comment 9

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8398575 [details] [diff] [review]
Fix failure in testSecurity/testSafeBrowsingNotificationBar.js (

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Incorrect merging of bug 985623 on beta
User impact if declined: its-a-trap test phishing links won't work, 985623 still won't be fixed on beta 
Testing completed (on m-c, etc.): m-b by skupin setting flags
Risk to taking this patch (and alternatives if risky): 
String or IDL/UUID changes made by this patch: none

Comment 10

[[:mmc] Monica Chew (no longer reading bugmail)]

> Risk to taking this patch (and alternatives if risky): 

Oops, forgot this field. Alternatives are backing out https://bugzilla.mozilla.org/show_bug.cgi?id=985623 on beta, which is less risky than taking this patch.

> String or IDL/UUID changes made by this patch: none

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8398575 [details] [diff] [review]
Fix failure in testSecurity/testSafeBrowsingNotificationBar.js (

Here it is. I hope I didn't do anything wrong while changing the product/component!

Comment 12

[[:mmc] Monica Chew (no longer reading bugmail)]

Thanks! I also built and tested manually with going to http://www.mozilla.org/firefox/its-a-trap.html which shows the interstitial with this patch.

https://hg.mozilla.org/releases/mozilla-beta/rev/9617ccbb0f82

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2]]

It would have been better if that follow-up patch landed as part of the original bug. Now it's a bit harder to see the relationship, and where it belongs to. Also we lost our bug which handles the appropriate Mozmill test failure for this regression. Anyway, it's too late now. But thanks for the patch!

Please do not forget to set the target milestone and the status flags for patches landed. I assume this one is done now? So lets even close it as fixed.

Comment 14

[Ioana (away)]

According to mozmill-release.blargon7.com, this test still displays one failure on Linux systems:
http://mozmill-release.blargon7.com/#/remote/failure?app=All&branch=All&platform=All&from=2014-03-31&to=2014-04-01&test=%2FtestSecurity%2FtestSafeBrowsingNotificationBar.js&func=testNotificationBar

The current fail message is different though: "controller.waitForPageLoad(URI=about:blank, readyState=uninitialized)".

For all the other runs on beta 4, the test passed.