811246 - Need to test page-mod attachTo comprehensively

Status: RESOLVED INCOMPLETE

(Add-on SDK Graveyard :: General, defect)

Description

[Erik Vold [:erikvold] (please needinfo? me)]

We test mainly the positive cases for attachTo, and not the negative cases.  In other words we mainly only test that attachTo X attaches to X but we are not usually testing that attachTo X is not applied to Y or Z.

Example:

We aren't testing that attachTo: ['top'] is not attached to existing frames (this may actually be tested but this is just a example).

Comment 1

[Erik Vold [:erikvold] (please needinfo? me)]

Also because of the way that the test are currently written it is very hard to know where the holes are..  there should be a more systematic approach.

Comment 2

[Sunil]

Curious to know if there's a bug in the attachTo functionality 'frame', where sometimes a frame gets mistakenly identified as 'top' and script gets incorrectly attached. This is especially when contentScriptWhen is specified as 'start', when all the data structures aren't correctly setup.

I have seen this happens sometimes where my script gets attached to Facebook/Google widget. I don't have an isolated test case yet.

Comment 3

[Matteo Ferretti [:zer0] [:matteo]]

(In reply to Sunil from comment #2)

> Curious to know if there's a bug in the attachTo functionality 'frame',
> where sometimes a frame gets mistakenly identified as 'top' and script gets
> incorrectly attached. This is especially when contentScriptWhen is specified
> as 'start', when all the data structures aren't correctly setup.
> 
> I have seen this happens sometimes where my script gets attached to
> Facebook/Google widget. I don't have an isolated test case yet.

We check if the document is a top document using the following condition: `window.top === window`.
So, if for any reason the `top` is the same as the current window even for iframe – I would assume a bug in platform or something – then the scripts are attached.

If you can't provide an isolate test case, I would suggest to you to modify the SDK itself, using your customized version to run on your add-ons and see if it's happening again. So, instead of this line:

  let isTopDocument = window.top === window;
  (https://github.com/mozilla/addon-sdk/blob/master/lib/sdk/page-mod.js#L216)

You could try something different like:

   let isTopDocument = window.frameElement === null;

For example. If you don't notice the error again, we could update our check. You could also put some `console.log` to debug a bit. The cause could be different, that's why I'm suggesting some log anyway.

Of course, if you will be able to isolate a test case, we can work on that instead.

Comment 4

[Andy McKay]

Because of the difficulty finding mentors and the expected life span of the SDK, we are removing [good first bug] from remaining SDK bugs.

Comment 5

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1399562