Last Comment Bug 684047 - page-mod: contentScripts being injected in all frames of a page
: page-mod: contentScripts being injected in all frames of a page
Status: RESOLVED FIXED
:
Product: Add-on SDK
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
P2 normal (vote)
: 1.11
Assigned To: Alexandre Poirot [:ochameau] (Back on 26th)
:
:
Mentors:
: 739929 (view as bug list)
Depends on: 708190
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-01 14:25 PDT by Hax App
Modified: 2012-10-04 07:27 PDT (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Pull request 541 (165 bytes, text/html)
2012-08-29 15:26 PDT, Alexandre Poirot [:ochameau] (Back on 26th)
rFobic: review+
Details

Description User image Hax App 2011-09-01 14:25:55 PDT
User Agent: Mozilla/5.0 (Windows NT 6.1; rv:6.0) Gecko/20100101 Firefox/6.0
Build ID: 20110811165603

Steps to reproduce:

Created a page-mod to inject contenScripts in every page and noticed all scripts being injected in all frames of that page.


Actual results:

It does exactly as it says on the tin, contentScripts are injected in every iframe by default, causing all scripts to load as many times as frames are in the page. Visiting pages like amazon, techcrunch or any other with many ads as iframes make the scripts load for every iframe. Worse yet, if one of the scripts is jquery (around 100k) it starts to eat memory and slow the page down, being loaded as many as 30 times in techcrucnh.


Expected results:

It depends, if that is the intended behavior. But there should be an option to load contentScripts only in the top frame (default), all of them, or as indicated in a frame url filter. Unaware rocketeers (most of us) injecting heavy scripts may not realize their scripts are slowing down firefox and increasing memory consumption.
Comment 1 User image Myk Melez [:myk] [@mykmelez] 2011-09-08 11:15:23 PDT
This is intended behavior, but indeed, it does seem like there should be an option to make content scripts only run on the topmost frame of a page, since addon authors are likely to want that behavior on a regular basis.
Comment 2 User image Jeff Griffiths (:canuckistani) (:⚡︎) 2011-09-08 21:33:31 PDT
I commented on a recent question on SOF related to this bug:

http://stackoverflow.com/questions/7338880/how-to-filter-out-iframes-in-a-firefox-jetpack-extension

As Erik hints at in his answer, we would be implementing the userscript '@noframes' metadata flag in some way.
Comment 3 User image Erik Vold 2011-12-06 17:01:29 PST
I'll implement some sort of @noframes equivalent for page-mods
Comment 4 User image Myk Melez [:myk] [@mykmelez] 2011-12-14 11:16:25 PST
In an recent email conversation with a couple folks about this, I proposed the following API:

We add an optional `contentScriptWhere` option to content script-enabled APIs (Page Mods, Page Worker, Panel, Widget) that determines which windows of a target page can have content scripts attached to them. The option can have three possible values:

    "top": only the topmost window can have content scripts attached to it
    "frames": only frame windows can have content scripts attached to them
    ["top", "frames"]: both the topmost window and frames can have content scripts attached to them
Comment 5 User image Myk Melez [:myk] [@mykmelez] 2011-12-14 11:17:05 PST
Also:

For Page Mods, `contentScriptWhere` specifications are still subject to filtering by the `include` value, so topmost and frame URLs still have to match that value to have content scripts attached to them.
Comment 6 User image Michael Balazs 2012-04-03 16:46:52 PDT
Seems this could be related to/worked on in conjunction with 739929: Filter page-mod by frame parent in addition to URL
Comment 7 User image Irakli Gozalishvili [:irakli] [:gozala] [@gozala] 2012-05-03 11:46:49 PDT
*** Bug 739929 has been marked as a duplicate of this bug. ***
Comment 8 User image Michael Balazs 2012-08-29 10:07:32 PDT
Given the resolution to 708190, what are the plans for this bug and the near duplicate 739929. 

Thanks for the great work.
Comment 9 User image Alexandre Poirot [:ochameau] (Back on 26th) 2012-08-29 15:26:58 PDT
Created attachment 656641 [details]
Pull request 541

That should come soon, hopefully for 1.11, as other page-mod patches.
Here is a work in progress, documentation is missing and some more unit tests.
Comment 10 User image Irakli Gozalishvili [:irakli] [:gozala] [@gozala] 2012-08-31 02:10:09 PDT
Comment on attachment 656641 [details]
Pull request 541

Few comments, but no need to make another review.
Comment 11 User image [github robot] 2012-08-31 06:47:18 PDT
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/57a6332efd3c9ac82f71c3e674b74c6b6b0f561c
Merge pull request #541: Bug 684047: Implements PageMod.attachTo: [top, frame]  in order to apply only to top-level documents.

Note You need to log in before you can comment on or make changes to this bug.