Closed Bug 633221 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox :: Shell Integration, defect, P2)

x86
Linux
defect

Tracking

()

VERIFIED FIXED
Firefox 4.0b12
Tracking Status
blocking2.0 --- -

People

(Reporter: george.carstoiu, Assigned: ttaubert)

References

Details

(Keywords: regression)

Attachments

(3 files, 3 obsolete files)

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
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?
Status: NEW → UNCONFIRMED
Ever confirmed: false
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.
Attached image Screenshot 1
Attached image Screenshot 2
Able to reproduce using Mozilla/5.0 (X11; Linux i686; rv:2.0b12pre) Gecko/20110210 Firefox/4.0b12pre.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Blocks: 585689
blocking2.0: --- → ?
Priority: -- → P2
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
http://hg.mozilla.org/mozilla-central/rev/f518eb4285bf (bug 624267)

Looks like this is the source of the regression.
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]
Summary: Panorama breaks when FF is set as default browser → Setting FF as default browser throws error (and breaks panorama)
Blocks: 626527
Component: TabCandy → General
Keywords: regression
QA Contact: tabcandy → general
Summary: Setting FF as default browser throws error (and breaks panorama) → Setting FF as default browser throws error (and breaks session restore)
Moving to Shell Integration. Sorry for the bug spam.
Component: General → Shell Integration
QA Contact: general → shell.integration
Blocks: 624267
No longer blocks: 585689, 626527
Assignee: nobody → tim.taubert
Status: NEW → ASSIGNED
Summary: Setting FF as default browser throws error (and breaks session restore) → Setting FF as default browser throws error when GIO Service is not available
Attached patch patch v1 (obsolete) — Splinter Review
Attachment #512042 - Flags: review?(roc)
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=tim.taubert@gmx.de&rev=074eb56a41e9
Attachment #512042 - Flags: approval2.0?
Attached patch patch for checkin (obsolete) — Splinter Review
Attachment #512042 - Attachment is obsolete: true
Glad you got approval, because while this kinda hurts, I don't think we should hold the release for it at this stage. blocking-.
blocking2.0: ? → -
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
If everything's ok after restarting Firefox then I guess that this is the problem this patch will fix.
http://hg.mozilla.org/mozilla-central/rev/5b2e1e0e6335
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b12
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);
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
(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=tim.taubert@gmx.de&rev=4eaceb0ecfb5
Attached patch patch v2 (obsolete) — Splinter Review
Test corrected. Patch rewritten by comment #18.
Attachment #512103 - Attachment is obsolete: true
Attachment #512202 - Flags: review?(roc)
Attachment #512202 - Flags: approval2.0?
Attachment #512202 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/4982e1298b23
Status: ASSIGNED → RESOLVED
Closed: 9 years ago9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Works for me on:
Mozilla/5.0 (X11; Linux i686; rv:2.0b12) Gecko/20100101 Firefox/4.0b12
Considering comment 25 setting to Verified Fixed. 

Thanks for checking Andrei.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.