575524 - browser_bug562890.js fails with other application extensions installed

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Description

[Phil Ringnalda (:philor)]

From http://tinderbox.mozilla.org/showbuilds.cgi?tree=Firefox-Release building the Fx 4.0b1 release:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Release/1277788094.1277788755.32742.gz
linux64_test mochitest-other on 2010/06/28 22:08:14

and

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Release/1277787145.1277787651.28643.gz
linux_test mochitest-other on 2010/06/28 21:52:25

and

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Release/1277791355.1277792129.13935.gz
win32_test mochitest-other on 2010/06/28 23:02:35

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/mozapps/extensions/test/browser/browser_bug562890.js | Prefs button should be shown for addon with a optionsURL set - Got true, expected false
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/mozapps/extensions/test/browser/browser_bug562890.js | Timed out
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/mozapps/extensions/test/browser/browser_bug562890.js | Found an unexpected tab at the end of test run: about:addons

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Release/1277789415.1277790112.5978.gz
macosx64_test mochitest-other on 2010/06/28 22:30:15

(same failures as above, followed by every test that checks active installs failing like

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/mozapps/extensions/test/xpinstall/browser_auth.js | Should be no active installs at the end of the test - Got 1, expected 0

plus

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/mozapps/plugins/tests/browser_bug435788.js | Should be just one install - Got 2, expected 1
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/mozapps/plugins/tests/browser_bug435788.js | Should be fully installed - Got 3, expected 6
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/mozapps/plugins/tests/browser_bug435788.js | Should have installed the extension - Got [e-mail], expected [e-mail]">[e-mail]
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/mozapps/plugins/tests/browser_bug435788.js | Should be one active installs - Got 2, expected 1
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/mozapps/plugins/tests/browser_bug435788.js | Should not be any installs - Got 1, expected 0)

Two variables: it's 4.0b1 rather than 4.0b1pre, and it's --enable-official-branding rather than Minefield.

FWIW, the test passes for me in 4.0b2pre + official branding.

Don't actually know how we handle test failures on release builds, so I'm just guessing it's a P1 blocker rather than a normal whatever (at the very least, until we know whether it's the test failing, or what the test is supposed to test failing)

Comment 1

[Nick Thomas [:nthomas] (UTC+12)]

Also
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Release/1277793108.1277793703.20018.gz
macosx_test mochitest-other on 2010/06/28 23:31:48
so that's all platforms.

Comment 2

[Robert Strong (they/them - no direct email)]

Possibly similar to Bug 573246? Perhaps browser_bug562890.js wasn't verified to work with the Feedback extension installed.

Comment 3

[Robert Strong (they/them - no direct email)]

I highly suspect that is the case since the test uses
var addonItem = addonList.childNodes[0];

and the add-on name is "Test add-on 1". If the Feedback extension does not start with a character that would be after Test when sorted alphabetically it would end up being returned by the above code.

Comment 4

[Robert Strong (they/them - no direct email)]

Attachment: possible patch - untested

I need to recompile before I can test this. Also note that browser_bug562890 has PC instead of UNIX line endings which should be fixed at the same time... I'll attach a separate patch for this so review is easier.

Comment 5

[Robert Strong (they/them - no direct email)]

Attachment: fix line endings

Comment 6

[Robert Strong (they/them - no direct email)]

btw: I am taking 1/2 day PTO tomorrow so if this needs to land please do so.

Comment 7

[Dave Townsend [:mossop]]

Surprised the Seamonkey guys didn't pick this up sooner. This landed after I did all the tests with Feedback so missed it. There doesn't seem to be anything troubling about the failure except that it is possible we are masking failures in future tests I guess.

Comment 8

[Dave Townsend [:mossop]]

Comment on attachment 454808 [details] [diff] [review]
possible patch - untested

Looks ok for now.

>diff --git a/toolkit/mozapps/extensions/test/browser/browser_bug562890.js b/toolkit/mozapps/extensions/test/browser/browser_bug562890.js

>-    addonItem = addonList.childNodes[1];
>+    for (i = 0; i < addonList.childNodes.length; i++) {
>+      var addonItem = addonList.childNodes[i];

No need to redeclare the var here.

>+      if (addonItem.hasAttribute("name") &&
>+          addonItem.getAttribute("name") == "Test add-on 2")
>+        break;
>+    }

Comment 9

[Robert Strong (they/them - no direct email)]

Attachment: patch w/ comment addressed

carrying forward r+

Comment 10

[Robert Strong (they/them - no direct email)]

Attachment: fix line endings

Would be a good thing to fix the line endings in this file at the same time.

Comment 11

[Robert Strong (they/them - no direct email)]

I also verified the patch fixes this bug by running the tests with an additional add-on with a name that starts with a in the app dir's extensions directory. I'm going to be away from the computer for several hours so feel free to land (preferably after the line endings review).

Comment 12

[Dave Townsend [:mossop]]

Comment on attachment 454900 [details] [diff] [review]
fix line endings

Yep, was just going to land this too

Comment 13

[Dave Townsend [:mossop]]

Landed on trunk: http://hg.mozilla.org/mozilla-central/rev/4980d2c1252c
And on the relbranch: http://hg.mozilla.org/mozilla-central/rev/2ef0a5ac123b

Comment 14

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

No more failures since the patch has been landed. Marking as verified fixed.