Skip to content

Commit

Permalink
Make self url detection work when loaded in primary window as well
Browse files Browse the repository at this point in the history
Our self-url (`BOOMR.url`) detection logic only worked when we loaded in an
iframe because the logic checks for `w.parent !== w`.  This fails if boomerang
is loaded in the primary window (eg using a static script or async loader or
link rel="preload").

This patch fixes this logic:
   1. Move the self-url detection logic out of the `if (w.parent !== w)` block
   2. Use `document.currentScript` to check for self url, which should work
      for all browsers except IE even if we don't have an `id` set on the script
   3. Use `document.getElementById("boomr-scr-as")` as well, which our new preloader
      loader will use to load boomerang into the primary window.

As a side-effect, boomerang will now correctly detect its API KEY even if loaded in
a non-standard manner (except for IE) as we use `document.currentScript`.

Additionally, the following bugs in unit tests were fixed:
   - Restiming tests that did not exclude boomerang & config urls from the check.

     This worked in the past because when loading boomerang in the parent window,
     our self-url check failed, so we weren't excluding boomerang from the RT trie.
     After fixing the above, these tests started failing since we were now doing
     the right thing. The tests have now been fixed to also do the right thing.

   - BOOMR_mq tests that were not using `BOOMR.window.` to dereference `BOOMR_mq`

     It is not clear why these tests failed without `BOOMR.window`, this also only
     appears to happen on headless chrome, which runs the tests on jenkins.
  • Loading branch information
bluesmoon authored and nicjansma committed Apr 4, 2018
1 parent b1dd31b commit fc44746
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 16 deletions.
3 changes: 2 additions & 1 deletion boomerang.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,10 @@ BOOMR_check_doc_domain();
document.getElementById("boomr-if-as") &&
document.getElementById("boomr-if-as").nodeName.toLowerCase() === "script") {
w = w.parent;
myurl = document.getElementById("boomr-if-as").src;
}

myurl = (document.currentScript || document.getElementById("boomr-if-as") || document.getElementById("boomr-scr-as")).src;

d = w.document;

// Short namespace because I don't want to keep typing BOOMERANG
Expand Down
4 changes: 2 additions & 2 deletions tests/page-templates/11-restiming/06-iframes.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ describe("e2e/11-restiming/06-iframes", function() {
var url = pageResources[i].name;

// ideally, we should skip anything in RT that is newer than our beacon
// skip beacon URL
if (url.indexOf(BOOMR.getBeaconURL()) !== -1) {
// skip beacon, boomerang, & config URLs
if (url.indexOf(BOOMR.getBeaconURL()) !== -1 || url === BOOMR.url || url === BOOMR.config_url) {
continue;
}
// skip favicon which is requested after beacon
Expand Down
4 changes: 2 additions & 2 deletions tests/page-templates/11-restiming/08-cross-origin.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ describe("e2e/11-restiming/06-iframes", function() {
for (var i = 0; i < pageResources.length; i++) {
var url = pageResources[i].name;

// skip beacon URL
if (url.indexOf("beacon") !== -1) {
// skip beacon, boomerang, & config URLs
if (url.indexOf(BOOMR.getBeaconURL()) !== -1 || url === BOOMR.url || url === BOOMR.config_url) {
continue;
}

Expand Down
22 changes: 11 additions & 11 deletions tests/unit/04-plugins-mq.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe("BOOMR.plugins.mq", function() {
describe("push()", function() {
it("Should handle degenerate cases", function() {
assert.doesNotThrow(function() {
BOOMR_mq.push(
BOOMR.window.BOOMR_mq.push(
null,
undefined,
false,
Expand All @@ -44,14 +44,14 @@ describe("BOOMR.plugins.mq", function() {
BOOMR.method = function() {
done();
};
BOOMR_mq.push(["method"]);
BOOMR.window.BOOMR_mq.push(["method"]);
});

it("Should call namespaced methods on BOOMR", function(done) {
BOOMR.method = function() {
done();
};
BOOMR_mq.push(["BOOMR.method"]);
BOOMR.window.BOOMR_mq.push(["BOOMR.method"]);
});

it("Should pass all arguments", function(done) {
Expand All @@ -62,7 +62,7 @@ describe("BOOMR.plugins.mq", function() {
assert.equal(arguments[2], 2);
done();
};
BOOMR_mq.push(["method", 0, 1, 2]);
BOOMR.window.BOOMR_mq.push(["method", 0, 1, 2]);
});

it("Should support `push` with multiple arguments", function(done) {
Expand All @@ -79,7 +79,7 @@ describe("BOOMR.plugins.mq", function() {
assert.equal(results[1], "method2");
done();
};
BOOMR_mq.push(
BOOMR.window.BOOMR_mq.push(
["method1"],
["method2"],
["method3"]
Expand All @@ -92,15 +92,15 @@ describe("BOOMR.plugins.mq", function() {
done();
}
};
BOOMR_mq.push(["obj.method"]);
BOOMR.window.BOOMR_mq.push(["obj.method"]);
});

it("Should step into functions on BOOMR", function(done) {
BOOMR.func = function() {};
BOOMR.func.method = function() {
done();
};
BOOMR_mq.push(["func.method"]);
BOOMR.window.BOOMR_mq.push(["func.method"]);
});

it("Should use appropriate context", function(done) {
Expand All @@ -112,7 +112,7 @@ describe("BOOMR.plugins.mq", function() {
done();
}
};
BOOMR_mq.push(["obj.method1"]);
BOOMR.window.BOOMR_mq.push(["obj.method1"]);
});
});

Expand All @@ -121,15 +121,15 @@ describe("BOOMR.plugins.mq", function() {
BOOMR.method = function() {
done();
};
BOOMR_mq.push({
BOOMR.window.BOOMR_mq.push({
arguments: ["method"]
});
});
it("Should support `callback`", function(done) {
BOOMR.method = function() {
return 123;
};
BOOMR_mq.push({
BOOMR.window.BOOMR_mq.push({
arguments: ["method"],
callback: function() {
assert.lengthOf(arguments, 1);
Expand All @@ -148,7 +148,7 @@ describe("BOOMR.plugins.mq", function() {
};

BOOMR.method = function() {};
BOOMR_mq.push({
BOOMR.window.BOOMR_mq.push({
arguments: ["method"],
callback: Item.prototype.callback,
thisArg: new Item(123)
Expand Down

0 comments on commit fc44746

Please sign in to comment.