Closed
Bug 935753
Opened 11 years ago
Closed 11 years ago
Firefox displays the "This is a secure Firefox page" indicator on pages served by addons
Categories
(Firefox :: General, defect)
Tracking
()
RESOLVED
FIXED
Firefox 28
People
(Reporter: amloessb, Assigned: jaws)
References
Details
Attachments
(2 files)
11.78 KB,
image/png
|
Details | |
1.63 KB,
patch
|
MattN
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 (Beta/Release) Build ID: 20131025151332 Steps to reproduce: I use the IT Tab addon (https://addons.mozilla.org/en-US/firefox/addon/ie-tab-2-ff-36/) to view pages on my corporate intranet using the Internet Explorer rendering engine. Actual results: I noticed today after updating to Firefox 25 that these pages displayed an orange Firefox emblem in the URL bar and showed "This is a secure Firefox page" when the emblem was clicked (see attached screenshot). Expected results: I do not expect to see this emblem on these pages, since they are in no way under the control of Mozilla. Even if an addon is rendering the page, it could still contain anything.
Comment 1•11 years ago
|
||
Erp. Sounds like something is confused, but I thought we had this UI whitelisted to only certain about: pages... I wonder if IE Tab is reusing one inappropriately?
Reporter | ||
Comment 2•11 years ago
|
||
More detailed reproduction steps: 1. Open a website (e.g., www.gmail.com) in a new tab. 2. Click the IE Tab toolbar button to switch to the IE rendering engine. 3. Observe the presence of the Firefox address bar badge.
Assignee | ||
Comment 3•11 years ago
|
||
We don't have a whitelist, but we should add one. The code that determines if this UI is shown is at http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#6491.
Assignee: nobody → jaws
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86_64 → All
Assignee | ||
Comment 4•11 years ago
|
||
This is the list of about: pages shown on about:about: about: about:about about:addons about:app-manager about:buildconfig about:cache about:config about:crashes about:credits about:customizing about:downloads about:healthreport about:home about:license about:logo about:memory about:mozilla about:networking about:newtab about:permissions about:plugins about:preferences about:privatebrowsing about:rights about:robots about:sessionrestore about:support about:sync-log about:sync-progress about:sync-tabs about:telemetry about:welcomeback Of the above pages, I think we should limit this identity to pages that a "secure" identifier will benefit the user. For example, about:mozilla doesn't benefit from a positive security signal. I would also like to limit the list to high-profile pages, which would reduce the size of the list to hopefully make it more manageable. I'd like to limit this identity to the following pages: about:addons about:app-manager about:config about:crashes about:healthreport about:home about:permissions about:preferences about:sessionrestore about:support about:welcomeback (about:newtab is not included here because it is treated as a "blank" tab)
Assignee | ||
Updated•11 years ago
|
Component: Untriaged → General
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #829355 -
Flags: review?(MattN+bmo)
Comment 6•11 years ago
|
||
Comment on attachment 829355 [details] [diff] [review] Patch Review of attachment 829355 [details] [diff] [review]: ----------------------------------------------------------------- Whitelists make me sad when they get out-of-date, but I can see why we don't want to show this UI for all third-party about:* pages. My other idea would be to build a list of all about: pages by default at compile-time (via xpcshell) and then use a blacklist to exclude the smaller number we don't want. This is probably fine for now since we don't add new highly-visible about: pages *that* often. r=me with consideration for the two additions below and the comment fix. Sorry for the delay. ::: browser/base/content/browser.js @@ +6483,3 @@ > > // For some URIs like data: we can't get a host and so can't do > // anything useful here. Chrome URIs however get special treatment. I think you should move "Chrome URIs however get special treatment." above the whitelist and indicate what this list represents (i.e. rationale from comment 4). For example: "Some chrome URIs are whitelisted to provide a positive security signal to the user:" @@ +6490,5 @@ > > + let chromeWhitelist = ["about:addons", "about:app-manager", "about:config", > + "about:crashes", "about:healthreport", "about:home", > + "about:permissions", "about:preferences", "about:sessionstore", > + "about:support", "about:welcomeback"]; I would probably add "about:newaddon" (one of the ones excluded from about:about[1]) so users understand that it's Firefox asking if they want to install the add-on. I also wonder about "about:privatebrowsing" since it has a similar role to about:home in that it's the initial page for private browsing mode. [1] https://mxr.mozilla.org/mozilla-central/ident?i=HIDE_FROM_ABOUTABOUT
Attachment #829355 -
Flags: review?(MattN+bmo) → review+
Assignee | ||
Comment 7•11 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/430ebb28dd92
Comment 8•11 years ago
|
||
Backed out for mochitest-bc failures. https://hg.mozilla.org/integration/fx-team/rev/1fbf3444ee50 https://tbpl.mozilla.org/php/getParsedLog.php?id=30776585&tree=Fx-Team
Assignee | ||
Comment 9•11 years ago
|
||
Relanded with test fix and comment, https://hg.mozilla.org/integration/fx-team/rev/6fae9d6feec8
Assignee | ||
Comment 10•11 years ago
|
||
And pushed a follow-up because I missed the test comment to update, https://hg.mozilla.org/integration/fx-team/rev/bf562f4eb546
Comment 11•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6fae9d6feec8 https://hg.mozilla.org/mozilla-central/rev/bf562f4eb546
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
Comment 12•11 years ago
|
||
Should this fix be uplifted?
You need to log in
before you can comment on or make changes to this bug.
Description
•