Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Prevent a webpage document element to be detected as the Extension API object #153

Merged
merged 5 commits into from Aug 15, 2018
Merged

fix: Prevent a webpage document element to be detected as the Extension API object #153

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c8cbd6e
fix: Prevent a webpage document element to be detected as the Extensi…
rpl Jul 21, 2018
57da1c3
chore: applied changes based on the last review comments
rpl Aug 15, 2018
a6f31b2
chore: applied a small tweak to fix a test unit failure due to the la…
rpl Aug 15, 2018
de4adf5
chore: added integration for API object detection in background page …
rpl Aug 15, 2018
17536d2
chore: minor tweaks on tests based on last round of review comments
rpl Aug 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions test/fixtures/detect-existing-browser-api-object/background.html
View file
Open in desktop
@@ -0,0 +1,20 @@
<!DOCTYPE>
<html>
<head>
<title>Test Extension Background page</title>
<meta charset="utf-8">
</head>
<body>
<!--
The following two DOM elements ensure that the polyfill doesn't detect the
globals defined for these DOM element ids as the Extensions API objects.
-->
<div id="browser"></div>
<div id="browser"></div>
<div id="chrome"></div>

<script src="copy-original-api-objects.js"></script>
<script src="browser-polyfill.js"></script>
<script src="background.js"></script>
</body>
</html>
15 changes: 15 additions & 0 deletions test/fixtures/detect-existing-browser-api-object/background.js
View file
Open in desktop
@@ -0,0 +1,15 @@
/* global originalAPIObjects */

// Register an additional message handler that always reply after
// a small latency time.
browser.runtime.onMessage.addListener(async (msg, sender) => {
if (msg !== "test-api-object-in-background-page") {
return false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why return false;? If you don't want the handler to respond, jus return void (return;).
Or make this a non-async function and call sendResponse({ ... }); in the end.

}

return Promise.resolve({
browserIsDefined: !!browser,
chromeIsDefined: !!chrome,
browserIsUnchanged: browser === originalAPIObjects.browser,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's check window.browser too?

});
});
17 changes: 16 additions & 1 deletion ...r-api-object-in-content-script/content.js → ...ct-existing-browser-api-object/content.js
View file
Open in desktop
@@ -1,10 +1,12 @@
/* global originalAPIObjects */

test("browser api object in content script", (t) => {
t.ok(browser && browser.runtime, "a global browser API object should be defined");
t.ok(chrome && chrome.runtime, "a global chrome API object should be defined");

if (navigator.userAgent.includes("Firefox/")) {
// Check that the polyfill didn't create a polyfill wrapped browser API object on Firefox.
t.equal(browser.runtime, chrome.runtime, "browser.runtime and chrome.runtime should be equal on Firefox");
t.equal(browser, originalAPIObjects.browser, "browser API object should not be changed on Firefox");
// On Firefox, window is not the global object for content scripts, and so we expect window.browser to not
// be defined.
t.equal(window.browser, undefined, "window.browser is expected to be undefined on Firefox");
Expand All @@ -16,3 +18,16 @@ test("browser api object in content script", (t) => {
t.equal(browser, window.browser, "browser and window.browser should be the same object");
}
});

test("browser api object in background page", async (t) => {
const reply = await browser.runtime.sendMessage("test-api-object-in-background-page");

t.ok(reply.browserIsDefined, "a global browser API object should be defined");
t.ok(reply.chromeIsDefined, "a global chrome API object should be defined");

if (navigator.userAgent.includes("Firefox/")) {
t.ok(reply.browserIsUnchanged, "browser API object should not be changed on Firefox");
} else {
t.ok(!reply.browserIsUnchanged, "browser API object should have been defined by the polyfill");
}
});
2 changes: 2 additions & 0 deletions test/fixtures/detect-existing-browser-api-object/copy-original-api-objects.js
View file
Open in desktop
@@ -0,0 +1,2 @@
// Store a copy of the references to the original API objects.
const originalAPIObjects = {browser, chrome}; // eslint-disable-line no-unused-vars
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment that in browsers without a browser API, this will point to the <div id="browser"> in the page.

4 changes: 4 additions & 0 deletions ...pi-object-in-content-script/manifest.json → ...existing-browser-api-object/manifest.json
View file
Open in desktop
Expand Up @@ -3,12 +3,16 @@
"name": "test-detect-browser-api-object-in-content-script",
"version": "0.1",
"description": "test-detect-browser-api-object-in-content-script",
"background": {
"page": "background.html"
},
"content_scripts": [
{
"matches": [
"http://localhost/*"
],
"js": [
"copy-original-api-objects.js",
"browser-polyfill.js",
"tape.js",
"content.js"
Expand Down
4 changes: 2 additions & 2 deletions test/integration/test-extensions-in-browser.js
View file
Open in desktop
Expand Up @@ -18,8 +18,8 @@ defineExtensionTests({
});

defineExtensionTests({
description: "polyfill should detect an existent browser API object in content scripts",
extensions: ["detect-browser-api-object-in-content-script"],
description: "polyfill should detect an existing browser API object in content scripts and extension pages",
extensions: ["detect-existing-browser-api-object"],
});

defineExtensionTests({
Expand Down