Port kinto blocklist preference changes to Thunderbird to fix XPCShell and Mozmill test failure

RESOLVED FIXED in Thunderbird 48.0

Status

Thunderbird
Preferences
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: Stefan Sitter, Assigned: Jorg K (GMT+2))

Tracking

48 Branch
Thunderbird 48.0

Firefox Tracking Flags

(Not tracked)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

a year ago
Comm-central currently has many Mozmill and XPCShell test failues related to kinto and blocklist. Common error seems to be:

> INFO -  NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getCharPref]
> @resource://gre/modules/services-common/KintoBlocklist.js:170:3

Looks like they all fail because the new preferences added with Bug 1257556 cannot be read in Thunderbird.
(Reporter)

Updated

a year ago
Depends on: 1257556
Indeed, the changes introduced in Bug 1257556 removed the lazy lookup of the blocklist prefs:

https://dxr.mozilla.org/mozilla-central/rev/0891f0fa044cba28024849803e170ed7700e01e0/services/common/KintoBlocklist.js#164

My fault.

I can draft a patch now, but cannot go further before monday unfortunately.
Created attachment 8744420 [details]
MozReview Request: Bug 1266794 - Fix lazy load of blocklist collections preferences

Review commit: https://reviewboard.mozilla.org/r/48533/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48533/
Comment on attachment 8744420 [details]
MozReview Request: Bug 1266794 - Fix lazy load of blocklist collections preferences

Made the changes without testing. I let you take over if necessary to fix it before monday.
Attachment #8744420 - Flags: feedback?(ssitter)
Attachment #8744420 - Flags: feedback?(MattN+bmo)
Comment on attachment 8744420 [details]
MozReview Request: Bug 1266794 - Fix lazy load of blocklist collections preferences

This is fine with me but I'm not sure it's really fixing the problem. Does TB want to use these lists or not? If so, the pref should be added instead.
Attachment #8744420 - Flags: feedback?(MattN+bmo) → feedback+
I realize the current patch won't work because `client.collectionName` is a public property accessed elsewhere.

Will have to patch this properly.
Attachment #8744420 - Flags: review-
(Assignee)

Comment 6

a year ago
(In reply to Matthew N. [:MattN] (behind on reviews) from comment #4)
> This is fine with me but I'm not sure it's really fixing the problem. Does
> TB want to use these lists or not? If so, the pref should be added instead.
Sadly we don't know much about the issue. If it doesn't hurt, we could just add whatever preference is needed. These, right?
pref("services.kinto.addons.collection", "addons");
pref("services.kinto.addons.checked", 0);
pref("services.kinto.plugins.collection", "plugins");
pref("services.kinto.plugins.checked", 0);
pref("services.kinto.gfx.collection", "gfx");
pref("services.kinto.gfx.checked", 0);

That's what we've done in bug 1247662 (https://hg.mozilla.org/comm-central/rev/57ada76a9c40).

We also asked in bug 1248557, but so far this has gone nowhere. Or am I confusing issues here?
(Assignee)

Comment 7

a year ago
Created attachment 8744574 [details] [diff] [review]
Adding more kinto preferences.

Here's a try run:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a92fcffac065
(Assignee)

Comment 8

a year ago
Comment on attachment 8744574 [details] [diff] [review]
Adding more kinto preferences.

The try came out green apart from the failures from bug 1266823.

Should we land this and the follow up in bug 1248557 as planned.
Attachment #8744574 - Flags: review?(mkmelin+mozilla)

Comment 9

a year ago
Mathieu or Matthew: Is there any documentation on the Mozilla Wiki or elsewhere what the function of the individual kinto preferences and their relation to the OneCRL settings is?

If I understand correctly, each application has to pick its level of support cafeteria-style from the list of available collections, but that means that Thunderbird and SeaMonkey have to keep track when new collections pop up and set their prefs accordingly in order to pick it up, right?
(Reporter)

Updated

a year ago
Attachment #8744420 - Flags: feedback?(ssitter)

Comment 10

a year ago
Comment on attachment 8744574 [details] [diff] [review]
Adding more kinto preferences.

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

Yes, looks like the prefs are the same as the Firefox version. I don't know if we use this for anything yet. But shouldn't hurt to be the same.
Attachment #8744574 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 11

a year ago
https://hg.mozilla.org/comm-central/rev/9ebe6f187f59d48f7fae551390d0a55a7c88f7e6

Updated

a year ago
Assignee: nobody → mozilla
Status: NEW → RESOLVED
Last Resolved: a year ago
Keywords: checkin-needed
OS: Unspecified → All
Hardware: Unspecified → All
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 48.0
10 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* comm-central: 10

Platform breakdown:
* windowsxp: 4
* windows7-32: 4
* osx-10-6: 1
* osx-10-10: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1266794&startday=2016-04-18&endday=2016-04-24&tree=all
(Quoting rsx11m from comment #9)
> Mathieu or Matthew: Is there any documentation on the Mozilla Wiki or
> elsewhere what the function of the individual kinto preferences and their
> relation to the OneCRL settings is?
> 
> If I understand correctly, each application has to pick its level of support
> cafeteria-style from the list of available collections, but that means that
> Thunderbird and SeaMonkey have to keep track when new collections pop up and
> set their prefs accordingly in order to pick it up, right?

Mathieu, can you look into whether this was an appropriate solution and answer the questions. If there are things in toolkit which "need" Kinto, those prefs should probably be moved to all.js so they work with other Toolkit applications.
Flags: needinfo?(mathieu)
(In reply to Matthew N. [:MattN] (behind on reviews) from comment #13)
> (Quoting rsx11m from comment #9)
> > Mathieu or Matthew: Is there any documentation on the Mozilla Wiki or
> > elsewhere what the function of the individual kinto preferences and their
> > relation to the OneCRL settings is?
> > 
> > If I understand correctly, each application has to pick its level of support
> > cafeteria-style from the list of available collections, but that means that
> > Thunderbird and SeaMonkey have to keep track when new collections pop up and
> > set their prefs accordingly in order to pick it up, right?
> 
> Mathieu, can you look into whether this was an appropriate solution and
> answer the questions. If there are things in toolkit which "need" Kinto,
> those prefs should probably be moved to all.js so they work with other
> Toolkit applications.

I got confirmation from :mgoodwin: we should move the preferences to `all.js`.

I will take care of this in Bug 1266235 if appropriate.

With regards to documentation, something was started here: https://wiki.mozilla.org/Firefox/Kinto but it would deserve a lot more attention.
Flags: needinfo?(mathieu)
(Assignee)

Comment 15

a year ago
We've been waiting for Mark Goodwin's answer for two months now in bug 1248557 comment #1 although he had offered his help in bug 1247662 comment #9.

Comment 16

a year ago
(In reply to Mathieu Leplatre (:leplatrem) from comment #14)
> (In reply to Matthew N. [:MattN] (behind on reviews) from comment #13)
> > Mathieu, can you look into whether this was an appropriate solution and
> > answer the questions. If there are things in toolkit which "need" Kinto,
> > those prefs should probably be moved to all.js so they work with other
> > Toolkit applications.
> 
> I got confirmation from :mgoodwin: we should move the preferences to
> `all.js`.
> 
> I will take care of this in Bug 1266235 if appropriate.

Sounds good, if it's supposed to be the same for all applications, that's the best place and avoids guesswork on our side which prefs need to be set to make it work properly. If an application doesn't want the Kinto updates, they can always set services.kinto.update_enabled to false in an override.

> With regards to documentation, something was started here:
> https://wiki.mozilla.org/Firefox/Kinto but it would deserve a lot more
> attention.

Thanks for your detailed explanation in bug 1248557 comment #7, that was the high-level description I was looking for. So, assuming that we are still in "Phase 1" that would imply testing to be restricted to just making sure it's connecting to the server without actually performing any blocking actions yet.
You need to log in before you can comment on or make changes to this bug.