page-mod documentation should have an example using contentScriptFile

RESOLVED FIXED in 1.1

Status

Add-on SDK
Documentation
P1
normal
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: dietrich, Assigned: wbamberg)

Tracking

unspecified

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [cherry-pick-1.1])

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
Because the construction of local file URLs is non-obvious and non-intuitive.
OS: Mac OS X → All
Priority: -- → P2
Hardware: x86 → All
Target Milestone: --- → 1.0
(automatic reprioritization of 1.0 bugs)
Priority: P2 → P1
Target Milestone: 1.0 → 1.1
(Assignee)

Comment 2

6 years ago
Created attachment 551917 [details] [diff] [review]
Added example using contentScriptFile
Assignee: nobody → wbamberg
Attachment #551917 - Flags: review?(dietrich)
(Reporter)

Comment 3

6 years ago
Comment on attachment 551917 [details] [diff] [review]
Added example using contentScriptFile

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

r=me w/ these minor changes.

::: packages/addon-kit/docs/page-mod.md
@@ +63,5 @@
> +### Using `contentScriptFile` ###
> +
> +For conciseness the examples above, and the other examples in this page,
> +create content scripts as strings and use the `contentScript` property to
> +assign them.

This sentence is awkward. Maybe break it into two somehow?

@@ +77,5 @@
> +code like:
> +
> +    const data = require("self").data;
> +    
> +    var pageMod = require("page-mod");

This example uses const for 'data' but var for 'pageMod'. I think we should be consistent about how we bring external APIs into the current scope. But that doesn't need to be fixed just here, so it's up to you if you want to change this or not.
Attachment #551917 - Flags: review?(dietrich) → review+
(Assignee)

Comment 4

6 years ago
Thanks Dietrich! Landed as:
https://github.com/mozilla/addon-sdk/commit/0d9f3b8778b29a8d6de572230cdaf58803c7d1d2

> > +    const data = require("self").data;
> > +    
> > +    var pageMod = require("page-mod");
> 
> This example uses const for 'data' but var for 'pageMod'. I think we should
> be consistent about how we bring external APIs into the current scope. But
> that doesn't need to be fixed just here, so it's up to you if you want to
> change this or not.

It's a good catch: we decided we should var in all the examples, so I fixed the other ones in this file too.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [cherry-pick-1.1]
You need to log in before you can comment on or make changes to this bug.