Strongly advise against using contentScript rather than contentScriptFile for complex scripts.

RESOLVED FIXED

Status

Add-on SDK
Documentation
P1
major
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: kmag, Assigned: wbamberg)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
The page-mod documentation should advise that contentScriptFile should be used rather than contentScript for complex scripts and that it should never be used with non-static strings. 

We get a lot of submissions to AMO with improperly sanitized, non-static strings used for content scripts (and elsewhere), and a fair number with unreadable, unformated, multiply concatenated strings. The former are a bug-prone security hazard, and the latter are nearly impossible to review and which can't be validated by our validator. I've rejected a lot of add-ons for using non-static strings, and wasted quite a lot of reviewing the the complex strings before giving a warning that it needs to be changed.
(Assignee)

Updated

7 years ago
Assignee: nobody → wbamberg

Updated

7 years ago
Priority: -- → P1
(Reporter)

Comment 1

7 years ago
Bumping to major since I'm rejecting quite a lot of add-ons for this.
Severity: normal → major
(Assignee)

Comment 2

7 years ago
Created attachment 588232 [details] [diff] [review]
big red warning
Attachment #588232 - Flags: review?(dietrich)
Comment on attachment 588232 [details] [diff] [review]
big red warning

Review of attachment 588232 [details] [diff] [review]:
-----------------------------------------------------------------

r=me on this change.

I do wonder, however, if we should include a warning about the consequences of not heeding the warning - something about problems getting add-on approval on AMO.
Attachment #588232 - Flags: review?(dietrich) → review+
(Assignee)

Comment 4

7 years ago
Created attachment 589767 [details] [diff] [review]
Reworded warning to mention AMO approval

Sorry to ask again, but I thought it was worth another check. I also made the warning a bit less jarring.
Attachment #588232 - Attachment is obsolete: true
Attachment #589767 - Flags: review?(dietrich)
Attachment #589767 - Flags: review?(dietrich) → review+

Comment 5

7 years ago
Commit pushed to https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/c6f71643d58285fdf7bf11af0dc05c8507fbc871
Bug 713605 - Strongly advise against using contentScript rather than contentScriptFile for complex scripts.; r=@dietrich
(Assignee)

Updated

7 years ago
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Comment 6

7 years ago
Commit pushed to https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/7152df115768d271a881ba74fcbc411ecb482c26
Bug 713605 - Strongly advise against using contentScript rather than contentScriptFile for complex scripts.; r=@dietrich
(cherry picked from commit c6f71643d58285fdf7bf11af0dc05c8507fbc871)
You need to log in before you can comment on or make changes to this bug.