Closed
Bug 1051847
Opened 10 years ago
Closed 10 years ago
Add trusted identity block to remaining end-user facing about: pages
Categories
(Firefox :: General, defect)
Tracking
()
People
(Reporter: sevaan, Assigned: viv1fi, Mentored)
References
Details
(Whiteboard: [lang=js][good first bug])
Attachments
(2 files, 3 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
viv1fi
:
review+
|
Details | Diff | Splinter Review |
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+
Comment 2•10 years ago
|
||
suggestedfix |
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•10 years ago
|
Points: --- → 3
Comment 3•10 years ago
|
||
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
Whiteboard: [lang=js][good first bug]
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).
Updated•10 years ago
|
Assignee: nobody → viv1fi
Comment 6•10 years ago
|
||
(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.
Updated•10 years ago
|
Attachment #8502077 -
Flags: review?(nfroyd)
Comment 8•10 years ago
|
||
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•10 years ago
|
Flags: qe-verify?
Comment 9•10 years ago
|
||
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•10 years ago
|
||
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`.
Comment 11•10 years ago
|
||
(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•10 years ago
|
||
Thanks for your feedback.
Attachment #8502077 -
Attachment is obsolete: true
Attachment #8503449 -
Attachment is obsolete: true
Attachment #8503471 -
Flags: review+
Comment 13•10 years ago
|
||
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 14•10 years ago
|
||
Comment on attachment 8503471 [details] [diff] [review] conservative.patch looks good, thanks
Attachment #8503471 -
Flags: review?(dao) → review+
Updated•10 years ago
|
Keywords: checkin-needed
Comment 15•10 years ago
|
||
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•10 years ago
|
||
Like this?
Attachment #8503471 -
Attachment is obsolete: true
Attachment #8505839 -
Flags: review+
Comment 17•10 years ago
|
||
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
Comment 18•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/659522f48d69
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Updated•10 years ago
|
Iteration: --- → 36.1
Comment 19•10 years ago
|
||
(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•10 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"
Comment 21•10 years ago
|
||
(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)
Comment 22•10 years ago
|
||
Filed Bug 1094947 to add trusted identity block to about:downloads page
Flags: needinfo?(valeriopetroni)
Updated•10 years ago
|
Flags: qe-verify? → qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•