1223371 - remove windows search, aka integrated search, from Thunderbird System integration dialog startup prompt

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Wayne Mery (:wsmwk)]

We should remove this from startup and not tempt the user to use it:
* No advice is offered about how to use it or it's potential performance impact
* It is not a performance feature - sometimes we encounter users with severe performance issues which are found to be caused by this being enabled, more often than not the user doesn't remember enabling it (and thus never even used it)
* Per bug 574525 comment 7, MS doesn't offer it in newer OS.

I'd cite examples of performance issues but I've lost the relevant firefox browsing sessions and I'm not going to attempt to find the links again.

I don't curently have an opinion regarding Mac Spotlight.

Comment 1

[:aceman]

What do you mean by "startup" ? I know the Windows Search option resides in the "System integration dialog". So what is meant here?

Comment 2

[Wayne Mery (:wsmwk)]

(In reply to :aceman from comment #1)
> What do you mean by "startup" ? I know the Windows Search option resides in
> the "System integration dialog". So what is meant here?

Thank you. Yes, precisely the integration prompt.  I don't know about Mac performance impact, so if someone decides this is only to be done for Windows users that's fine.

And the sooner we do this is removed the better. (even though back in the day I was very much in favor of offering this feature in the prompt)  If could set a blocking flag for TB52 I would do it.

Comment 3

[:aceman]

Do we want to limit it to some Windows version (like 7), or remove altogether? If we remove it, do we also turn it off?
You propose to only hide it from the UI for now, not all the backend code?

Comment 4

[Wayne Mery (:wsmwk)]

My proposal is to remove it for all Windows versions. But only from the integration dialog, not removing UI from preferences, and not disable it for those who have it enabled.

Comment 5

[:aceman]

Attachment: patch

This hides the option when the dialog is not opened from Options (so e.g. on startup), unless the search is already enabled (so user can turn it off).

Did the Windows search integration removal from Windows 8 materialize? Should we hide the option on these OS versions altogether? It seems Windows search does exist up to Windows 10, does it mean we just can't integrate (set it up for TB) with it?

Comment 6

[Wayne Mery (:wsmwk)]

(In reply to :aceman from comment #5)
> 
> Did the Windows search integration removal from Windows 8 materialize?
> Should we hide the option on these OS versions altogether? It seems Windows
> search does exist up to Windows 10, does it mean we just can't integrate
> (set it up for TB) with it?

Thanks for thinking of this.  I don't have an answer readily available.  Should probably be handled/asked in a separate bug.

Comment 7

[Magnus Melin [:mkmelin]]

https://en.wikipedia.org/wiki/Windows_Search#Windows_Desktop_Search says

Included with:
Wndows Vista
Windows Server 2008
Windows 7
Windows Server 2008 R2
Windows 8
Windows 8.1
Windows Server 2012
Windows Server 2012 R2
Windows 10

Comment 8

[Wayne Mery (:wsmwk)]

> Should we hide the option on these OS versions altogether? 

For the integration prompt, yes. All Windows versions.

Comment 9

[:aceman]

I meant "hide altogether" was also in the Options dialog.
But as it is supported up to 10 for now, we have nothing to do.

So with this patch, we show the option on Vista and up, but only in the Options subdialog (system integration), not the first start prompt.

On OS X the search is offered in all cases for now.

Comment 10

[Jorg K (CEST = GMT+2)]

Comment on attachment 8773610 [details] [diff] [review]
patch

Review of attachment 8773610 [details] [diff] [review]:
-----------------------------------------------------------------

I haven't tried it, but the patch looks reasonable. r=jorgk.

Comment 11

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/9b7ccf1f2bdd
Landed after testing it. I moved one brace to the line above ;-)

Comment 12

[Wayne Mery (:wsmwk)]

Aceman, is patch easy to uplift to 45?

bug 1307156 is an example of another affected user.

Comment 13

[:aceman]

I'd guess it can be applied to 45 as is. There probably wasn't any activity in this file.

Comment 14

[Wayne Mery (:wsmwk)]

Comment on attachment 8773610 [details] [diff] [review]
patch

[Approval Request Comment]
Regression caused by (bug #): -
User impact if declined: users tend to enable the feature without knowing what it does and how to use it, because it sounds like a good thing
Testing completed (on c-c, etc.): It's been on aurora
Risk to taking this patch (and alternatives if risky): low - affects first startup only new users or users creating a new profile

Comment 15

[Jorg K (CEST = GMT+2)]

Comment on attachment 8773610 [details] [diff] [review]
patch

Will we be doing another TB 50 beta where this would be included?

Comment 16

[Wayne Mery (:wsmwk)]

(In reply to Jorg K (GMT+2) from comment #15)
> Comment on attachment 8773610 [details] [diff] [review]
> patch
> 
> Will we be doing another TB 50 beta where this would be included?

I would like to in about a week, yes. I haven't mentioned it yet on TB drivers.

Comment 17

[Jorg K (CEST = GMT+2)]

Beta (TB 50):
https://hg.mozilla.org/releases/comm-beta/rev/e694081295e389f28a11cde94c4d4a1cc69050c9

Comment 18

[Kent James (:rkent)]

Comment on attachment 8773610 [details] [diff] [review]
patch

Review of attachment 8773610 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/content/systemIntegrationDialog.js
@@ +76,5 @@
>      if (this.SearchIntegration)
>      {
>        this._searchCheckbox.checked = this.SearchIntegration.prefEnabled;
> +      // On Windows, do not offer the option on startup as it does not perform well.
> +      if ((Services.appinfo.OS == "WINNT") && !calledFromPrefs &&

This line does not work, Services is loaded at this.Services (See usage above. this.Services.prefs)

Comment 19

[Jorg K (CEST = GMT+2)]

Indeed. I tested it and the search check box is not displayed. But I didn't check the console where we can see:

JavaScript error: chrome://messenger/content/systemIntegrationDialog.js, line 80: ReferenceError: Services is not defined

Grrr.

Comment 20

[:aceman]

Grr, that's for blindly working for other platform...

Comment 21

[Kent James (:rkent)]

Comment on attachment 8773610 [details] [diff] [review]
patch

https://hg.mozilla.org/releases/comm-esr45/rev/d439d002b2ee