The default bug view has changed. See this FAQ.

page-mod: contentScripts being injected in all frames of a page

RESOLVED FIXED in 1.11

Status

Add-on SDK
General
P2
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Hax App, Assigned: ochameau)

Tracking

unspecified
1.11

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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.
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
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

5 years ago
I'll implement some sort of @noframes equivalent for page-mods
Assignee: nobody → erikvvold
Status: NEW → ASSIGNED
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
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

5 years ago
Seems this could be related to/worked on in conjunction with 739929: Filter page-mod by frame parent in addition to URL
Duplicate of this bug: 739929
Depends on: 708190
Assignee: erikvvold → evold
Assignee: evold → nobody

Comment 8

5 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

5 years ago
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.
Assignee: nobody → poirot.alex
(Assignee)

Updated

5 years ago
Attachment #656641 - Flags: review?(rFobic)
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

5 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

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.11
You need to log in before you can comment on or make changes to this bug.