The default bug view has changed. See this FAQ.

Add trusted identity block to remaining end-user facing about: pages

RESOLVED FIXED in Firefox 36

Status

()

Firefox
General
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: sevaan, Assigned: viv1fi, Mentored)

Tracking

Trunk
Firefox 36
x86
All
Points:
3
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [lang=js][good first bug])

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

3 years ago
Created attachment 8470862 [details]
Example of Identity Block

Some of the about: pages use an identity block to show the user that the page they are on
is a trusted part of the browser. The following about: pages do not use an identity block
(but should!):

about:
about:about
about:buildconfig
about:cache
about:downloads
about:license
about:logo
about:memory
about:mozilla
about:networking
about:newtab
about:plugins
about:rights
about:robots
about:sync-log
about:sync-progress
about:sync-tabs
about:telemetry
about:webrtc
Flags: firefox-backlog+

Updated

3 years ago
Blocks: 935753

Updated

3 years ago
Duplicate of this bug: 977752
This should be very simple since we can just add these to the whitelist that we have at http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#6572.

Updated

3 years ago
Points: --- → 3
Let's link to a specific file, so the whitelist link doesn't move around:

http://hg.mozilla.org/mozilla-central/file/4b180eaafdbf/browser/base/content/browser.js#l6379

I can mentor somebody here, even if I can't review.
Mentor: nfroyd@mozilla.com
Whiteboard: [lang=js][good first bug]

Comment 4

3 years ago
I think about:blank and about:newtab are the only internal pages that shouldn't have the identity block, since they aren't even "regular" pages from the user's point of view (they don't have a URL in the address bar).
Assignee: nobody → viv1fi
Assigning this to Sofie as part of her Ascend project work.
(In reply to jaramat from comment #4)
> I think about:blank and about:newtab are the only internal pages that
> shouldn't have the identity block, since they aren't even "regular" pages
> from the user's point of view (they don't have a URL in the address bar).

We have to be careful though of not showing this for add-on created about: pages. So we will still need to use a whiltelist.
Depends on: 988959
(Assignee)

Comment 7

3 years ago
Created attachment 8502077 [details] [diff] [review]
extended about: whitelist

Kept it a regex literal...
Attachment #8502077 - Flags: review?(nfroyd)
Comment on attachment 8502077 [details] [diff] [review]
extended about: whitelist

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

This looks good to me; turning final review over to Jared.
Attachment #8502077 - Flags: review?(nfroyd)
Attachment #8502077 - Flags: review?(jaws)
Attachment #8502077 - Flags: feedback+

Updated

3 years ago
Flags: qe-verify?
Comment on attachment 8502077 [details] [diff] [review]
extended about: whitelist

This whitelist intentionally doesn't contain all possible about: URIs but is supposed to contain those who are exposed to end users through some UI. Please extend it only to that end. We shouldn't let this regular expression grow liberally as this is performance-critical code.
Attachment #8502077 - Flags: review?(jaws) → review-
(Assignee)

Comment 10

3 years ago
Created attachment 8503449 [details] [diff] [review]
revised.patch

about:* w/ no UI:

buildconfig
cache
crashes
credits
downloads
license -- should have id block
logo
memory
mozilla
networking
newtab
plugins
rights -- should have id block
robots
sync-log
sync-progress
sync-tabs
webrtc

Any more conservative and I may as well remove `about:` & `about:about`.
(In reply to viv1fi from comment #10)
> Any more conservative and I may as well remove `about:` & `about:about`.

Yep, I see no reason why they should be part of this list.
(Assignee)

Comment 12

3 years ago
Created attachment 8503471 [details] [diff] [review]
conservative.patch

Thanks for your feedback.
Attachment #8502077 - Attachment is obsolete: true
Attachment #8503449 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8503471 - Flags: review+
Comment on attachment 8503471 [details] [diff] [review]
conservative.patch

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

Dao will still need to formally r+ this patch through Bugzilla. Thanks for working on this! :)
Attachment #8503471 - Flags: review+ → review?(dao)
Comment on attachment 8503471 [details] [diff] [review]
conservative.patch

looks good, thanks
Attachment #8503471 - Flags: review?(dao) → review+
Keywords: checkin-needed
Please attach a patch that includes commit information per the link below.
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Keywords: checkin-needed
(Assignee)

Comment 16

3 years ago
Created attachment 8505839 [details] [diff] [review]
0001-1051847-Added-ID-blocks-for-about-license-rights.patch

Like this?
Attachment #8503471 - Attachment is obsolete: true
Attachment #8505839 - Flags: review+
https://hg.mozilla.org/integration/fx-team/rev/659522f48d69
Summary: Add identity block to about: pages that do not have an identity block → Add trusted identity block to remaining end-user facing about: pages
https://hg.mozilla.org/mozilla-central/rev/659522f48d69
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36

Updated

3 years ago
Iteration: --- → 36.1
(In reply to viv1fi from comment #10)
> buildconfig
> crashes
> memory
> plugins

I think the patch that landed is fine, but for what it's worth, these ones are exposed indirectly via about:support.

Comment 20

2 years ago
The about:downloads page should be added to the whitelist because in private browsing it is exposed to end users through the download panel option "show all downloads"
(In reply to Valerio from comment #20)
> The about:downloads page should be added to the whitelist because in private
> browsing it is exposed to end users through the download panel option "show
> all downloads"

Please file a bug for this, good catch!
Flags: needinfo?(valeriopetroni)

Updated

2 years ago
Depends on: 1094947

Comment 22

2 years ago
Filed Bug 1094947 to add trusted identity block to about:downloads page
Flags: needinfo?(valeriopetroni)
Flags: qe-verify? → qe-verify-
You need to log in before you can comment on or make changes to this bug.