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

RESOLVED FIXED in Thunderbird 51.0

Status

RESOLVED FIXED
3 years ago
2 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)

(Reporter)

Description

3 years ago
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.
(Assignee)

Comment 1

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

Comment 2

2 years ago
(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
(Assignee)

Comment 3

2 years ago
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
(Reporter)

Comment 4

2 years ago
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.
(Assignee)

Comment 5

2 years ago
Created attachment 8773610 [details] [diff] [review]
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?
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Flags: needinfo?(sid.bugzilla)
Attachment #8773610 - Flags: feedback?(vseerror)
(Reporter)

Comment 6

2 years ago
(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)
(Assignee)

Updated

2 years ago
Attachment #8773610 - Flags: review?(mkmelin+mozilla)
(Reporter)

Updated

2 years ago
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
(Reporter)

Comment 8

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

For the integration prompt, yes. All Windows versions.
(Assignee)

Comment 9

2 years ago
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.
(Reporter)

Updated

2 years ago
User Story: (updated)

Comment 10

2 years ago
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+

Comment 11

2 years ago
https://hg.mozilla.org/comm-central/rev/9b7ccf1f2bdd
Landed after testing it. I moved one brace to the line above ;-)
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 51.0
(Reporter)

Comment 12

2 years ago
Aceman, is patch easy to uplift to 45?

bug 1307156 is an example of another affected user.
Flags: needinfo?(sid.bugzilla)
(Assignee)

Comment 13

2 years ago
I'd guess it can be applied to 45 as is. There probably wasn't any activity in this file.
(Reporter)

Comment 14

2 years ago
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 15

2 years ago
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+

Updated

2 years ago
status-thunderbird50: --- → fixed
status-thunderbird51: --- → fixed
status-thunderbird_esr45: --- → affected
tracking-thunderbird_esr45: --- → ?
(Reporter)

Comment 16

2 years ago
(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 18

2 years ago
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)

Updated

2 years ago
Blocks: 1311821

Updated

2 years ago
No longer blocks: 1311821
Depends on: 1311821

Comment 19

2 years ago
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.
(Assignee)

Comment 20

2 years ago
Grr, that's for blindly working for other platform...

Updated

2 years ago
status-thunderbird_esr45: affected → fixed
tracking-thunderbird_esr45: ? → 50+
(Assignee)

Updated

2 years ago
Depends on: 1321469
(Assignee)

Updated

2 years ago
Depends on: 1314236
You need to log in before you can comment on or make changes to this bug.