Closed
Bug 684047
Opened 13 years ago
Closed 12 years ago
page-mod: contentScripts being injected in all frames of a page
Categories
(Add-on SDK Graveyard :: General, defect, P2)
Add-on SDK Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
1.11
People
(Reporter: haxapp, Assigned: ochameau)
References
Details
Attachments
(1 file)
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•13 years ago
|
||
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
Comment 2•13 years ago
|
||
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.
I'll implement some sort of @noframes equivalent for page-mods
Assignee: nobody → erikvvold
Status: NEW → ASSIGNED
Comment 4•12 years ago
|
||
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•12 years ago
|
||
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•12 years ago
|
||
Seems this could be related to/worked on in conjunction with 739929: Filter page-mod by frame parent in addition to URL
Updated•12 years ago
|
Assignee: erikvvold → evold
Updated•12 years ago
|
Assignee: evold → nobody
Comment 8•12 years ago
|
||
Given the resolution to 708190, what are the plans for this bug and the near duplicate 739929. Thanks for the great work.
Assignee | ||
Comment 9•12 years ago
|
||
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.
Assignee: nobody → poirot.alex
Assignee | ||
Updated•12 years ago
|
Attachment #656641 -
Flags: review?(rFobic)
Comment 10•12 years ago
|
||
Comment on attachment 656641 [details]
Pull request 541
Few comments, but no need to make another review.
Attachment #656641 -
Flags: review?(rFobic) → review+
Comment 11•12 years ago
|
||
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.
Assignee | ||
Updated•12 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.11
You need to log in
before you can comment on or make changes to this bug.
Description
•