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

RESOLVED FIXED in Thunderbird 51.0

Status

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: wsmwk, Assigned: aceman)

Tracking

({perf})

Trunk
Thunderbird 51.0
All
Windows
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird50 fixed, thunderbird_esr4550+ fixed, thunderbird51 fixed)

Details

User Story

perf problem examples

https://support.mozilla.org/en-US/questions/988169#answer-581429
http://thomas-cokelaer.info/blog/2011/08/thunderbird-hangs-forever-when-copying-message-to-sent-folder/#comment-46832
https://bugzilla.mozilla.org/show_bug.cgi?id=1294985#c9

related:
bug 979893 - temp directory in non-standard location results in Windows Indexing on .eml files

Attachments

(1 attachment)

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.
What do you mean by "startup" ? I know the Windows Search option resides in the "System integration dialog". So what is meant here?
(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.
Keywords: perf
Summary: remove windows search, aka integrated search, from Thunderbird startup prompt → remove windows search, aka integrated search, from Thunderbird System integration dialog startup prompt
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?
OS: Unspecified → Windows
Hardware: Unspecified → All
Version: unspecified → Trunk
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.
Posted patch patchSplinter Review
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?
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Flags: needinfo?(sid.bugzilla)
Attachment #8773610 - Flags: feedback?(vseerror)
(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.
User Story: (updated)
Attachment #8773610 - Flags: review?(mkmelin+mozilla)
Attachment #8773610 - Flags: feedback?(vseerror) → feedback+
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
> Should we hide the option on these OS versions altogether? 

For the integration prompt, yes. All Windows versions.
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.
User Story: (updated)
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.
Attachment #8773610 - Flags: review?(mkmelin+mozilla) → review+
https://hg.mozilla.org/comm-central/rev/9b7ccf1f2bdd
Landed after testing it. I moved one brace to the line above ;-)
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 51.0
Aceman, is patch easy to uplift to 45?

bug 1307156 is an example of another affected user.
Flags: needinfo?(sid.bugzilla)
I'd guess it can be applied to 45 as is. There probably wasn't any activity in this file.
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
Attachment #8773610 - Flags: approval-comm-esr45?
Attachment #8773610 - Flags: approval-comm-beta?
Comment on attachment 8773610 [details] [diff] [review]
patch

Will we be doing another TB 50 beta where this would be included?
Attachment #8773610 - Flags: approval-comm-beta? → approval-comm-beta+
(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 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)
Blocks: 1311821
No longer blocks: 1311821
Depends on: 1311821
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.
Grr, that's for blindly working for other platform...
Depends on: 1321469
Depends on: 1314236
You need to log in before you can comment on or make changes to this bug.