Closed Bug 1266794 Opened 4 years ago Closed 4 years ago

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

Categories

(Thunderbird :: Preferences, defect)

48 Branch
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 48.0

People

(Reporter: ssitter, Assigned: jorgk-bmo)

References

Details

Attachments

(2 files)

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.
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.
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.
(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?
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)
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?
Attachment #8744420 - Flags: feedback?(ssitter)
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: nobody → mozilla
Status: NEW → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed
OS: Unspecified → All
Hardware: Unspecified → All
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 48.0
(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)
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.
(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.