Closed
Bug 1457745
Opened 6 years ago
Closed 6 years ago
TEST-UNEXPECTED-FAIL | xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_pluginInfoURL.js
Categories
(Thunderbird :: General, defect)
Thunderbird
General
Tracking
(thunderbird60 fixed, thunderbird61 fixed)
RESOLVED
FIXED
Thunderbird 61.0
People
(Reporter: jorgk-bmo, Unassigned)
Details
(Whiteboard: [Thunderbird-testfailure: X all])
Attachments
(1 file)
2.83 KB,
patch
|
jorgk-bmo
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
TEST-UNEXPECTED-FAIL | xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_pluginInfoURL.js | xpcshell return code: 0 TEST-UNEXPECTED-FAIL | xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_pluginInfoURL.js | test_infoURL_missing - [test_infoURL_missing : 74] Should be using fallback when no infoURL tag is available. - "https://blocklist.addons.mozilla.org/en-US/xpcshell/blocked/test_plugin_noInfoURL" === "https://blocked.cdn.mozilla.net/test_plugin_noInfoURL.html" M-C last good: 8b2c1fc3d6c348f053fe33a478fa3b1ddb M-C first bad: 08f68e2c892cadc4035ecbfbf3529f32d4 https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8b2c1fc3d6c348f053fe33a478fa3b1ddb&tochange=08f68e2c892cadc4035ecbfbf3529f32d4 Looks like bug 1456515 last changed test_pluginInfoURL.js. Gijs and Kris, what's going wrong in the M-C test when run in TB?
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(gijskruitbosch+bugs)
Reporter | ||
Updated•6 years ago
|
Whiteboard: [Thunderbird-testfailure: X all]
Comment 1•6 years ago
|
||
https://dxr.mozilla.org/mozilla-central/search?q=https%3A%2F%2Fblocked.cdn.mozilla.net%2F&=mozilla-central seems like this is not a test domain but the "normal" greprefs one. Presumably TB/SM override this? If TB/SM have a good reason to not use the CDN for blocklist items (:TheOne might know?) then presumably the way to fix this would be to update the test to pushPrefEnv a different value for that pref so we don't implicitly rely on the shipped value -- but off-hand it would seem to me that TB/SM should be using the same CDN. Andreas?
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(awagner)
Comment 2•6 years ago
|
||
I'm sorry I don't know what this bug is about. Can you please add more context?
Flags: needinfo?(awagner)
Comment 3•6 years ago
|
||
(In reply to Andreas Wagner [:TheOne] [use NI] from comment #2) > I'm sorry I don't know what this bug is about. Can you please add more > context? From where do you/we expect TB/SM to get the informational pages when items are blocklisted? On m-c, it seems we use `https://blocked.cdn.mozilla.net/` . TB seems to be using "https://blocklist.addons.mozilla.org/" . Is that expected?
Flags: needinfo?(awagner)
Reporter | ||
Comment 4•6 years ago
|
||
The bug is about the Xpcshell test test_pluginInfoURL.js failing when run in the Thunderbird environment. I think Gijs is suggesting that two(?) preferences are at play here: extensions.blocklist.detailsURL and extensions.blocklist.itemURL. We use pref("extensions.blocklist.detailsURL", "https://addons.mozilla.org/%LOCALE%/%APP%/blocked/"); pref("extensions.blocklist.itemURL", "https://blocklist.addons.mozilla.org/%LOCALE%/%APP%/blocked/%blockID%"); extensions.blocklist.url is different, too: pref("extensions.blocklist.url", "https://blocklist.addons.mozilla.org/blocklist/3/%APP_ID%/... Blame shows that they were changed in 2011 and 2014: https://hg.mozilla.org/comm-central/rev/728871c36cf7 https://hg.mozilla.org/comm-central/rev/69dd558de5f9 So should we just delete the prefs and go with what ever M-C does? How will that be in the future (RSN(TM)) when we run our own add-ons site? Personally, I'd like to have the test failure cleared, so how can we achieve this quickly while we're working on an overall strategy?
Flags: needinfo?(sancus)
Flags: needinfo?(gijskruitbosch+bugs)
Comment 5•6 years ago
|
||
(In reply to Jorg K (GMT+1) from comment #4) > The bug is about the Xpcshell test test_pluginInfoURL.js failing when run in > the Thunderbird environment. > > I think Gijs is suggesting that two(?) preferences are at play here: > extensions.blocklist.detailsURL and extensions.blocklist.itemURL. > > We use > pref("extensions.blocklist.detailsURL", > "https://addons.mozilla.org/%LOCALE%/%APP%/blocked/"); > pref("extensions.blocklist.itemURL", > "https://blocklist.addons.mozilla.org/%LOCALE%/%APP%/blocked/%blockID%"); > > extensions.blocklist.url is different, too: > pref("extensions.blocklist.url", > "https://blocklist.addons.mozilla.org/blocklist/3/%APP_ID%/... > > Blame shows that they were changed in 2011 and 2014: > https://hg.mozilla.org/comm-central/rev/728871c36cf7 > https://hg.mozilla.org/comm-central/rev/69dd558de5f9 > > So should we just delete the prefs and go with what ever M-C does? I don't know, that's why I'm asking Andreas... > How will > that be in the future (RSN(TM)) when we run our own add-ons site? > > Personally, I'd like to have the test failure cleared, so how can we achieve > this quickly while we're working on an overall strategy? I suggested how to fix this in comment #1... (In reply to :Gijs (he/him) from comment #1) > then presumably the way to fix this would be to update the test > to pushPrefEnv a different value for that pref so we don't implicitly rely > on the shipped value but given that m-c is using a CDN, I'm inclined to think TB should be using that too, unless Andreas suggests otherwise.
Flags: needinfo?(gijskruitbosch+bugs)
Comment 6•6 years ago
|
||
(In reply to :Gijs (he/him) from comment #3) > From where do you/we expect TB/SM to get the informational pages when items > are blocklisted? On m-c, it seems we use `https://blocked.cdn.mozilla.net/` > . TB seems to be using "https://blocklist.addons.mozilla.org/" . Is that > expected? I am not sure this is still working for Tb/Sm. The URL changed in https://bugzilla.mozilla.org/show_bug.cgi?id=1341332 and it seems like at some point we changed or dropped the redirects.
Comment 7•6 years ago
|
||
(In reply to Andreas Wagner [:TheOne] [use NI] from comment #6) > (In reply to :Gijs (he/him) from comment #3) > > > From where do you/we expect TB/SM to get the informational pages when items > > are blocklisted? On m-c, it seems we use `https://blocked.cdn.mozilla.net/` > > . TB seems to be using "https://blocklist.addons.mozilla.org/" . Is that > > expected? > > I am not sure this is still working for Tb/Sm. The URL changed in > https://bugzilla.mozilla.org/show_bug.cgi?id=1341332 and it seems like at > some point we changed or dropped the redirects. OK. Then I think TB should drop its prefs so it uses the m-c ones. AFAICT SM doesn't have this set of prefs in the first place, only TB...
Flags: needinfo?(awagner)
Comment 8•6 years ago
|
||
I am also just inferring from what I read in https://bugzilla.mozilla.org/show_bug.cgi?id=1341332 which I found from blaming that pref in hg). Maybe this was done around the move to kinto? The timing would fit. I don't have any further information and so I am not sure I am the right person to give guidance here, sorry. Perhaps :leplatrem knows more?
Updated•6 years ago
|
Flags: needinfo?(mathieu)
Reporter | ||
Comment 9•6 years ago
|
||
Removing those preferences, we will now use the Mozilla default from here: https://searchfox.org/mozilla-central/rev/08df4e6e11284186d477d7e5b0ae48483ecc979c/modules/libpref/init/all.js#2526 Services.kinto.* appear to be deprecated as well, so I removed them while I was here.
Attachment #8971969 -
Flags: review?(sancus)
Attachment #8971969 -
Flags: review?(mkmelin+mozilla)
Attachment #8971969 -
Flags: review?(mathieu)
Reporter | ||
Comment 10•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=71ab8f26581f17e39a1e08b2c611d2d81a7b0d9a
Comment 11•6 years ago
|
||
(In reply to Jorg K (GMT+1) from comment #4) > So should we just delete the prefs and go with what ever M-C does? How will > that be in the future (RSN(TM)) when we run our own add-ons site? > > Personally, I'd like to have the test failure cleared, so how can we achieve > this quickly while we're working on an overall strategy? These URLs will need to point at addons.thunderbird.net within the next couple of months, certainly. That doesn't necessarily impact any change now. It does mean that Mozilla will need to add an additional rewrite to thunderbird.net at the CDN, which I'm not certain how difficult it is to do, unless we are OK writing off versions that get this change as unable to connect to the blocklist. Maybe less important if it only goes in a couple betas/nightlies, but a problem if it's in 60.0 release. I could also put in redirects at live.thunderbird.net so that we only need to change these once, as long as redirects are acceptable here.
Flags: needinfo?(sancus)
Reporter | ||
Comment 12•6 years ago
|
||
Comment on attachment 8971969 [details] [diff] [review] 1457745-remove-bl-prefs.patch I've discussed this with Andrei on IRC. I'll just go ahead and land this as a bustage fix. Soon TB will need it's own blocklist URL, then we put the preferences back and also submit a patch to fix the test.
Flags: needinfo?(mathieu)
Attachment #8971969 -
Flags: review?(sancus)
Attachment #8971969 -
Flags: review?(mkmelin+mozilla)
Attachment #8971969 -
Flags: review?(mathieu)
Comment 13•6 years ago
|
||
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/323c8a1be19c removed unnecessary and faulty blocklist preferences. rs=bustage-fix
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Reporter | ||
Updated•6 years ago
|
Target Milestone: --- → Thunderbird 61.0
Reporter | ||
Updated•6 years ago
|
Attachment #8971969 -
Flags: approval-comm-beta+
Comment 14•6 years ago
|
||
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/df13f2b8f71c Follow-up: removed more deprecated kinto preferences. r=me DONTBUILD
Reporter | ||
Comment 15•6 years ago
|
||
For the record: We added these kinto prefs in Feb/March 2016: https://hg.mozilla.org/comm-central/rev/57ada76a9c40 https://hg.mozilla.org/comm-central/rev/9ebe6f187f59 https://hg.mozilla.org/comm-central/rev/16b7ef1fb002 M-C renamed and moved them from FF to lib in April of 2016: https://hg.mozilla.org/mozilla-central/rev/c6c57d394549 https://hg.mozilla.org/mozilla-central/rev/a5aad78f70ea So basically TB has been wrong for two years now, but since M-C's lib provided the correct preferences, no one noticed :-(
Reporter | ||
Comment 16•6 years ago
|
||
Beta (TB 60 beta 6): https://hg.mozilla.org/releases/comm-beta/rev/8b0b9dd6e49c50946aa71a6e9d11cdf76ae4f299 https://hg.mozilla.org/releases/comm-beta/rev/ccf242f1eb9a7f2886001a662d939a25aac1e39c
status-thunderbird60:
--- → fixed
status-thunderbird61:
--- → fixed
Comment 17•6 years ago
|
||
> Maybe this was done around the move to kinto? The timing would fit. I don't have any further information and so I am not sure I am the right person to give guidance here, sorry.
> Perhaps :leplatrem knows more?
Indeed, when the source of truth changed from AMO to Kinto we redirected the URL. However, I was pretty sure we had put a redirection :(
You need to log in
before you can comment on or make changes to this bug.
Description
•