956484 - Rewrite inline script/style in /mobile/android/chrome/content/aboutPrivateBrowsing.xhtml

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[desiradaniel2007]

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.

Comment 1

[Neil Bleasdale [:nbleasdale]]

Attachment: bug956484.patch

Thought I'd look at this one as no one's picked it up yet.

Comment 2

[Matt Brubeck (:mbrubeck)]

Comment on attachment 8358957 [details] [diff] [review]
bug956484.patch

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

Looks good.  Thanks!

Comment 3

[Neil Bleasdale [:nbleasdale]]

Attachment: bug956484.patch

Could someone add 'checkin-needed' to this? I don't have edit bugs privileges as yet.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/4ddb7a3d9cae

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4ddb7a3d9cae

Comment 6

[Ameya Bapat]

Hi. When I do about:privatebrowsing it shows some CSS as well.
<style type="text/css"><![CDATA[
      body.normal .showPrivate,
      body.private .showNormal {
        display: none;
      }
      body.appMenuButtonVisible .toolsMenu {
        display: none;
      }
      body.appMenuButtonInvisible .appMenuButton {
        display: none;
      }
    ]]></style>

Ideally this should also be kept in a diff css file the way we are putting any js code in separate file?
Am i correct or missing something?

Comment 7

[Daniel Veditz [:dveditz]]

Ideally the CSS would not be inline either, but the CSP prohibition on 'unsafe-inline' can be applied separately to scripts and style. CSP blocks both by default, but we may have enabled inline style since style injection is less likely to be harmful. Comment 0 only mentions script, the summary mentions both; I'm not sure which is intended.

Comment 8

[desiradaniel2007]

Separate styles too. Updating description.