Different way of checking if browser is defined?

[fstanis]

I found a peculiar bug with using webextension-polyfill in webpack: issue 5828.

Long story short, the polyfill guard confuses webpack in certain circumstances:

webextension-polyfill/src/browser-polyfill.js

Line 9 in 8914541

if (typeof browser === "undefined") {

This got me thinking, any reason why something simpler like if (!window.browser) { is not used here instead?

[rpl]

@fstanis That's really unfortunate :-(

but also: Thank you for letting us know!

we are going to evaluate if we should workaround it in the polyfill itself.

[JannesMeyer]

Hi,

I just came across the same problem, and it seems that the problem has been fixed. However the last release on npm was 6 months ago, which doesn't include the fix yet. Any idea when the next release will happen?

Thanks

[ExE-Boss]

I personally prefer using:

if (!("browser" in window)) {
    // [...]
}

because using window.browser directly when testing if it exists tends to print dereferencing an undefined reference warnings in the console.

Edit: Turns out that that causes the tests to fail because of jsdom/jsdom#1622:

webextension-polyfill/test/setup.js

Lines 44 to 50 in 6f9cfdf

	// Set (or reset) the browser property.
	if (browserObject) {
	window.browser = browserObject;
	} else {
	// TODO: change into `delete window.browser` once tmpvar/jsdom#1622 has been fixed.
	window.browser = undefined;
	}

But using this function works:

const isDefined = (obj, key) => {
  if (typeof obj !== "object" || obj === null) {
    return false;
  }
  return key in obj && typeof obj[key] !== "undefined";
};

Edit 2: Turns out that that doesn’t work either because of #114 (comment) and #115 (comment)

[tophf]

Currently this polyfill can be trivially disabled by adding a DOM element with id browser in the page - in case the polyfill runs as a content script when that element is already in the page, that is at the default state of "run_at".

This is possible because browsers create implicit global variables for such elements and of course their type is not undefined, it's object. I guess you can add a check for instanceof Node.

See https://crbug.com/865881
BTW it means the priority is somewhat more critical.

[varjolintu]

@tophf Thanks for linking the bug here. I was just about to make a new issue. It's a really annoying problem. At first I feared that you could extend the element prototype and override the browser.runtime for some nasty code but luckily that didn't work.

Many pages can have an element ID'd as browser and this indeed breaks the content script every time.

[rpl]

#153 fixed the issue mention in #68 (comment), #3 already cover the Edge use case and @Rob--W filed #156 for the remaining issue with webpack.ProvidePlugin.

I'm closing this as a duplicate of #156.

[fregante]

Note: version 0.9.0 supports ProvidePlugin now thanks to #351

plugins: [
  new ProvidePlugin({
    browser: "webextension-polyfill"
  })
]