633221 - Setting FF as default browser throws error when GIO Service is not available

Status: VERIFIED FIXED

(Firefox :: Shell Integration, defect, P2)

Description

[George Carstoiu]

Mozilla/5.0 (X11; Linux i686; rv:2.0b12pre) Gecko/20110210 Firefox/4.0b12pre

Reproducible: Always

Prerequisites: Set different browser as default browser of the OS

Steps to reproduce:
 1. On clean profile start FF
 2. When asked whether you want to set FF as default browser, press "Yes".
 3. Enter Panorama

Actual results:
 - panorama breaks 

Expected results:
 - the two tabs are shown and panorama acts normally 

Observation:
 - if user selects "No" at the browser default prompt, panorama does not break

Comment 1

[Michael Yoshitaka Erlewine [:mitcho]]

Cannot reproduce on Mac.

George, how exactly does Panorama break? Do you get anything in the error console? Do you know if this is a regression?

Comment 2

[George Carstoiu]

Before logging bug, I did try to reproduce issue on Windows and Mac but I couldn't, only on Linux.

Please see Screenshot 1 and Screenshot 2 to understand what I meant by Panorama breaks.

Comment 3

[George Carstoiu]

Attachment: Screenshot 1

Comment 4

[George Carstoiu]

Attachment: Screenshot 2

Comment 5

[Teodosia Pop]

Able to reproduce using Mozilla/5.0 (X11; Linux i686; rv:2.0b12pre) Gecko/20110210 Firefox/4.0b12pre.

Comment 6

[George Carstoiu]

Regression range:

Last good:
Mozilla/5.0 (X11; Linux i686; rv:2.0b10pre) Gecko/20110111 Firefox/4.0b10pre
http://hg.mozilla.org/mozilla-central/rev/4413ed6ba5a5

First bad
Mozilla/5.0 (X11; Linux i686; rv:2.0b10pre) Gecko/20110112 Firefox/4.0b10pre
http://hg.mozilla.org/mozilla-central/rev/c0e05d518f57

Push log:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=4413ed6ba5a5&tochange=c0e05d518f57

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

http://hg.mozilla.org/mozilla-central/rev/f518eb4285bf (bug 624267)

Looks like this is the source of the regression.

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

This is what I get in the error console right after saying "yes" to make FF my default browser:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80040154 (NS_ERROR_FACTORY_NOT_REGISTERED) [nsIShellService.setDefaultBrowser]"  nsresult: "0x80040154 (NS_ERROR_FACTORY_NOT_REGISTERED)"  location: "JS frame :: chrome://browser/content/browser.js :: delayedStartup :: line 6795"  data: no]

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

Moving to Shell Integration. Sorry for the bug spam.

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

This is the line that fails with NS_ERROR_FACTORY_NOT_REGISTERED:

http://mxr.mozilla.org/mozilla-central/source/browser/components/shell/src/nsGNOMEShellService.cpp#233

Comment 11

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v1

Comment 12

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 512042 [details] [diff] [review]
patch v1

Requesting approval for a simple patch with a test :)

Pushed to try: http://tbpl.mozilla.org/?tree=MozillaTry&pusher=[e-mail]&rev=074eb56a41e9

Comment 13

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch for checkin

Comment 14

[Dietrich Ayala (:dietrich)]

Glad you got approval, because while this kinda hurts, I don't think we should hold the release for it at this stage. blocking-.

Comment 15

[George Carstoiu]

I would also like to point out the following related to the setting FF as default browser, reproducible only in Linux.

When updating from 3.6.14 to a beta release (in my case FF 4 beta 11) and choosing "Yes" in the dialogue the following also happen:
 - bookmark toolbar does not display the bookmarked websites (although they are present in "Show all bookmars|Bookmark toolbar")
 - "Error console" entry from the menu disappears

Comment 16

[Tim Taubert [:ttaubert] (inactive)]

If everything's ok after restarting Firefox then I guess that this is the problem this patch will fix.

Comment 17

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/5b2e1e0e6335

Comment 18

[timeless]

ttaubert: this was a high churn patch for a really simple change.

Why didn't you just do this:

      nsCOMPtr<nsIStringBundleService> bundleService =
        do_GetService(NS_STRINGBUNDLE_CONTRACTID, &rv);
-      NS_ENSURE_SUCCESS(rv, rv);
+      NS_ENSURE_SUCCESS(rv, NS_OK);

Comment 19

[Marco Bonardo [:mak]]

Backed out due to permaorange m-oth on Linux
http://hg.mozilla.org/mozilla-central/rev/0c6d2cf554c7
http://hg.mozilla.org/mozilla-central/rev/c6e8f8d4468e

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1297697772.1297699032.8784.gz
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/shell/test/browser_633221.js | we're not the default browser

plus the test looks like not being run on Linux64

Comment 20

[Tim Taubert [:ttaubert] (inactive)]

(In reply to comment #19)
> plus the test looks like not being run on Linux64

Runs on Linux64 - I pushed to try to test some preconditions.

http://tbpl.mozilla.org/?tree=MozillaTry&pusher=[e-mail]&rev=4eaceb0ecfb5

Comment 21

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v2

Test corrected. Patch rewritten by comment #18.

Comment 22

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 512202 [details] [diff] [review]
patch v2

Passed try.

http://tbpl.mozilla.org/?tree=MozillaTry&pusher=[e-mail]&rev=15d4da2cd98e

Comment 23

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch for checkin

Comment 24

[Jacek Caban]

http://hg.mozilla.org/mozilla-central/rev/4982e1298b23

Comment 25

[AndreiD[QA]]

Works for me on:
Mozilla/5.0 (X11; Linux i686; rv:2.0b12) Gecko/20100101 Firefox/4.0b12

Comment 26

[George Carstoiu]

Considering comment 25 setting to Verified Fixed. 

Thanks for checking Andrei.