Closed Bug 825452 Opened 9 years ago Closed 9 years ago

system integration dialog says 'Skip integration' even when launched from preferences


(Thunderbird :: OS Integration, defect)

Not set


(Not tracked)

Thunderbird 22.0


(Reporter: aryx, Assigned: aceman)



(1 file)

The Integration dialog where the user can set Thunderbird as default client for mails, newsgroups etc. can open in two situations:
- automatically on startup when Thunderbird detects that it isn't the default client (and it should check for this)
- from the preferences panel 'Advanced' > 'General'

There is a 'Skip integration' button in the dialog which makes not really sense if launched from the preferences, so a conditional 'Cancel' / 'Skip integration' button would make sense (likely sent to the window as argument).
Yeah, this is my work. I think we already know if we are called from preferences. I'd need some clever string proposal from bwinton.
Assignee: nobody → acelists
Flags: needinfo?(bwinton)
What about "Cancel" if we're launched from preferences?
Flags: needinfo?(bwinton)
Attached patch patchSplinter Review
No problem.
Attachment #709531 - Flags: ui-review?(bwinton)
Attachment #709531 - Flags: feedback?(archaeopteryx)
Comment on attachment 709531 [details] [diff] [review]

Review of attachment 709531 [details] [diff] [review]:

Thank you for the patch.
Attachment #709531 - Flags: feedback?(archaeopteryx) → feedback+
Comment on attachment 709531 [details] [diff] [review]

I like it, ui-r=me!

>+++ b/mail/components/preferences/advanced.js
>@@ -1,16 +1,17 @@
> var gAdvancedPane = {
>   mPane: null,
>   mInitialized: false,
>+  mShellServiceWorking: false,

Do we need to make this change as part of this patch?  It seems unrelated, and I worry a little about the case where we check for the shell service before it's initialized, and then set it to notWorking, and never re-check…

>@@ -83,22 +86,18 @@ var gAdvancedPane = {
>   checkDefaultNow: function (aAppType)
>   {
>-    var nsIShellService = Components.interfaces.nsIShellService;
>-    var shellSvc;
>-    try {
>-      shellSvc = Components.classes[";1"]
>-                           .getService(nsIShellService);
>-    } catch (ex) { return; }
>+    if (!this.mShellServiceWorking)
>+      return;

(In particular, this case.)

I'll let the code reviewer trace into it, and make the final decision here, but I wanted to express my concern.

Attachment #709531 - Flags: ui-review?(bwinton) → ui-review+
Comment on attachment 709531 [details] [diff] [review]

Yeah, that change is not needed, it is just an optimization (catching of exceptions can be costly).
I think when the preferences dialog is opened the shell-service should be already initialized (from app startup time?). So caching the state should be fine while in the dialog.
Attachment #709531 - Flags: review?(mconley)
I don't know if this is the same bug or not, but every time I create a new profile, I get a System Integration prompt. OK, but all I want to do is to properly set the default client values, and ask that the dialog be accepted.

But it is not clear to me what I am supposed to do. "Set as Default" seems wrong, since I am explictly telling it to NOT set Earlybird as the default client. "Skip Integration" sounds like the same thing as "Cancel".

I suppose what I should do is to say "Set as Default" that it is NOT the default, which really seems strange to me. I am tempted to check the code to see what this is supposed to do, but how does the average user make sense of this?

What happened to plain old "OK" and "Cancel"? That I understand.
They were renamed as they were not clear. See bug 476426.
Comment on attachment 709531 [details] [diff] [review]

Review of attachment 709531 [details] [diff] [review]:

It's true that the optimization in here is orthogonal to the problem that we're trying to solve, but I also think it helps to clear up what's going on. I like it.

Thanks Archaeopteryx!
Attachment #709531 - Flags: review?(mconley) → review+
Keywords: checkin-needed
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 22.0
You need to log in before you can comment on or make changes to this bug.