Closed Bug 1519626 Opened 9 months ago Closed 9 months ago

test_ext_permission_warnings.js fails when run in Thunderbird

Categories

(WebExtensions :: General, enhancement, P2)

63 Branch
enhancement

Tracking

(firefox66 fixed)

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: jorgk, Assigned: jorgk)

References

Details

(Whiteboard: [permission])

Attachments

(1 file, 2 obsolete files)

+++ This bug was initially created as a clone of Bug #1504018 +++

The problem is that the test uses
const bundle = Services.strings.createBundle("chrome://browser/locale/browser.properties");

and that bundle is not available. I little unfortunate that browser has crept into toolkit here.

Andrew, feel free to r- and suggest a better solution.

Attachment #9036112 - Flags: review?(aswan)

Can you update the test code to import the following if the app is Thunderbird:

Services.strings.createBundle("chrome://messenger/locale/addons.properties");

The test relies on the following:
https://searchfox.org/comm-central/rev/ab062df16f6764dfe7adad09a21fc84a4af732d0/mail/locales/en-US/chrome/messenger/addons.properties#151-203

Flags: needinfo?(jorgk)

Thanks for the tip!

Assignee: nobody → jorgk
Attachment #9036112 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #9036112 - Flags: review?(aswan)
Flags: needinfo?(jorgk)
Attachment #9036117 - Flags: review?(rob)
Comment on attachment 9036117 [details] [diff] [review]
1519626-test_ext_permission_warnings-take-2.patch

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

::: toolkit/components/extensions/test/xpcshell/test_ext_permission_warnings.js
@@ +1,4 @@
>  "use strict";
>  
>  let {ExtensionTestCommon} = ChromeUtils.import("resource://testing-common/ExtensionTestCommon.jsm", {});
> +let {AppConstants} = ChromeUtils.import("resource://gre/modules/AppConstants.jsm", {});

Remove this line.
AppConstants is already imported via toolkit/components/extensions/test/xpcshell/head.js
Attachment #9036117 - Attachment is obsolete: true
Attachment #9036117 - Flags: review?(rob)
Attachment #9036123 - Flags: review?(rob)
Comment on attachment 9036123 [details] [diff] [review]
1519626-test_ext_permission_warnings-take-2.patch (v1b) - please change to r=robwu,aswan

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

Looks good to me. I'm not a peer, so let's also ask aswan for r+.
Attachment #9036123 - Flags: review?(rob)
Attachment #9036123 - Flags: review?(aswan)
Attachment #9036123 - Flags: review+
Comment on attachment 9036123 [details] [diff] [review]
1519626-test_ext_permission_warnings-take-2.patch (v1b) - please change to r=robwu,aswan

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

I guess even better would be creating a test-specific string bundle for the purpose of this test to break the dependency on non-toolkit assets, but this seems fine for now.
Attachment #9036123 - Flags: review?(aswan) → review+
Keywords: checkin-needed
Attachment #9036123 - Attachment description: 1519626-test_ext_permission_warnings-take-2.patch (v1b) → 1519626-test_ext_permission_warnings-take-2.patch (v1b) - please change to r=robwu,aswan

Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd4be420c44f
Use application specific string bundle in test_ext_permission_warnings.js. r=robwu,aswan

Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

Will this fix require manual validation? if yes please specify how exactly to correctly validate it, thanks!

Flags: needinfo?(jorgk)

Nothing to verify here, thanks for asking. The test now passes for Firefox and Thunderbird.

Flags: needinfo?(jorgk)
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.