Closed Bug 972316 Opened 12 years ago Closed 11 years ago

Move inline scripts and styles into separate file for b2g/chrome/content/shell.html

Categories

(Firefox OS Graveyard :: General, defect)

Product:
Firefox OS Graveyard
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
1.4 S2 (28feb)

People

(Reporter: freddy, Assigned: gasolin)

References

(Blocks 1 open bug)

Details

(Whiteboard: [mentor=mgoodwin@mozilla.com][lang=html][good first bug][lang=js])

Attachments

(2 files, 2 obsolete files)

nocss.png
122.69 KB, image/png
Details
972316-2.patch
5.66 KB, patch
fabrice
: review+
Details | Diff | Splinter Review
With the current plan to harden the security of Firefox, we want to disallow internal privileged pages to use inline JavaScript. Since their amount is fairly limited, we start this by rewriting them bit by bit. See also https://wiki.mozilla.org/Security/Inline_Scripts_and_Styles
I'd like take it a try
Assignee: nobody → gasolin
Attached image nocss.png —
Hi Mark, It's my first gecko patch, I've moved styles to separate css but not sure what's the correct path to link in shell.html tried link with `chrome://browser/content/shell.css` or `shell.css` but they both seems not work
Fred, did you add you css file to b2g/chrome/jar.mn ? Attaching your patch would help!
Thanks I don't know I have to add file there! I'll test soon. And I have to figure out how to submit a gecko patch :/
It works! I'll come out a patch soon.
Attached patch 972316.patch (obsolete) — — Splinter Review
this patch move js and css out from shell.html
Attachment #8376076 - Flags: review?(fabrice)
Attached patch 972316.patch (obsolete) — — Splinter Review
fix no new line
Attachment #8376076 - Attachment is obsolete: true
Attachment #8376076 - Flags: review?(fabrice)
Attachment #8376079 - Flags: review?(fabrice)
Comment on attachment 8376079 [details] [diff] [review] 972316.patch Review of attachment 8376079 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/chrome/content/shell.js @@ +539,5 @@ > type: 'system-first-paint' > }); > break; > + case 'unload': > + stop(); You want this.stop() ::: b2g/chrome/jar.mn @@ +12,5 @@ > * content/dbg-browser-actors.js (content/dbg-browser-actors.js) > * content/settings.js (content/settings.js) > * content/shell.html (content/shell.html) > * content/shell.js (content/shell.js) > +* content/shell.css (content/shell.css) No need for the '*' since this file doesn't need pre-processing.
Attachment #8376079 - Flags: review?(fabrice) → feedback+
Comment on attachment 8376079 [details] [diff] [review] 972316.patch Review of attachment 8376079 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/chrome/content/shell.html @@ +13,3 @@ > > <head> > + <link rel="stylesheet" href="chrome://b2g/content/shell.css" type="text/css" media="all" /> you can simply use href="shell.css"
Attached patch 972316-2.patch — — Splinter Review
fixed and tested in emulator
Attachment #8376156 - Flags: review?(fabrice)
Comment on attachment 8376156 [details] [diff] [review] 972316-2.patch Review of attachment 8376156 [details] [diff] [review]: ----------------------------------------------------------------- r=me thanks!
Attachment #8376156 - Flags: review?(fabrice) → review+
Attachment #8376079 - Attachment is obsolete: true
thanks!
Keywords: checkin-needed
I have filed bug 974435 to track additional work on inline styles caused by scripts in shell.js. Maybe you're interested in fixing this as well, Fred?
Depends on: 974435
https://hg.mozilla.org/mozilla-central/rev/47f9517744ea
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S2 (28feb)
@freddyb ya I think I can handle that :)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: