1156400 - Prefix and Unify Dom:Security console warning

Status: NEW

(Core :: DOM: Security, defect, P3)

Description

[Christoph Kerschbaumer [:ckerschb]]

Attachment: tracking_prot.png

For CSP, CORS and other security warnings displayed in the console we usually prefix them with the name of the security feature (see attached screenshot when browsing cnn.com).
Would be great if we also do that for Tracking Protection.

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Francois, what do you think - sounds good?

Comment 2

[François Marier [:francois]]

Sounds good to me.

Comment 3

[Tanvi Vyas[:tanvi]]

Here's a pointer to where the console warning gets logged:
https://mxr.mozilla.org/mozilla-central/source/netwerk/base/nsChannelClassifier.cpp#481

You will have to do a little more digging to see what "TrackingUriBlocked" maps to and how to get a prefix in.

Comment 4

[Tanvi Vyas[:tanvi]]

Taking a second look at this, the way CSP does prefixing is this:
https://mxr.mozilla.org/mozilla-central/source/dom/security/nsCSPUtils.cpp#81

It appends to the message string that is later passed to console->LogMessage.  That makes sense for CSP that has many different error messages that don't mention CSP specifically:
http://mxr.mozilla.org/mozilla-central/source/dom/locales/en-US/chrome/security/csp.properties

But Tracking Protection just has one warning that says "tracking protection" in it.  So I'm not sure if this prefix is necessary or redundant:
http://mxr.mozilla.org/mozilla-central/source/netwerk/locales/en-US/necko.properties#41

Same question for Mixed Content Blocker - should we prepend messages with "Mixed Content Blocker"?
http://mxr.mozilla.org/mozilla-central/source/dom/locales/en-US/chrome/security/security.properties#3
http://mxr.mozilla.org/mozilla-central/source/dom/locales/en-US/chrome/security/security.properties#29

Comment 5

[Steven Englehardt [:englehardt]]

Prepending the category does make it easy to scan down the console output and see what each message relates to, even if there is only a single message possible as there is with Tracking Protection.

As an alternative to doing this for each category separately, we could prepend the category sent with the message prior to output. (See: https://dxr.mozilla.org/mozilla-central/source/browser/devtools/webconsole/webconsole.js#4770). But these categories are less descriptive than the current ones (e.g. CSP instead of Content Security Policy).

I think it makes sense to just do the updates individually for tracking protection and mixed content -- thoughts?

Comment 6

[Tanvi Vyas[:tanvi]]

Looking at the insecure password warning, we have the same problem where there is no prefix (test page http://people.mozilla.org/~tvyas/password_test2.html)

So maybe the best fix is to: 
1) Update the category names (ex: change CSP to Content Security Policy)
https://dxr.mozilla.org/mozilla-central/source/browser/devtools/webconsole/webconsole.js#4770
2) Prefix the strings for all CATEGORY_SECURITY bugs with their category name
3) Remove the custom prefixing CSP: https://mxr.mozilla.org/mozilla-central/source/dom/security/nsCSPUtils.cpp#81

Comment 7

[François Marier [:francois]]

If we go with Tanvi's suggestion from comment 6, then I guess we should retitle the bug and move it back to DOM::Security since it would apply to CSP, CORS, mixed content and TP?

Comment 8

[Christoph Kerschbaumer [:ckerschb]]

Yeah, let's re-classify to Dom:Security. In fact it would be awesome if we unify our Console Warnings to use the same style of Prefix and warning style throughout the codebase.