Closed Bug 1570678 Opened 5 years ago Closed 4 years ago

Consider replacing the (i) icon as the fallback icon for "unknown identity"/"potentially trustworthy" pages

Categories

(Firefox :: Site Identity, enhancement, P2)

Product:
Firefox
Component:
Site Identity
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 79
Tracking Flags:
Tracking Status
firefox79 --- fixed

People

(Reporter: johannh, Assigned: krystle.salazar2, Mentored)

References

(Blocks 1 open bug, Regressed 1 open bug)

Details

(Whiteboard: [privacy-panel][skyline])

Attachments

(3 files, 2 obsolete files)

Potentially Secure Pages.png
3.84 MB, image/png
Details
document-16.svg
574 bytes, image/svg+xml
Details
Bug 1570678 - Replace (i) icon for a file icon on potentially trustworthy pages. r=johannh
47 bytes, text/x-phabricator-request
Details | Review

Currently, lacking a better alternative, we're showing the ⓘ icon in place of the lock icon on pages that have a "potentially secure" origin.

These pages are:

  • about: pages that do not get the chromeUI treatment
  • file://
  • neterror pages
  • resource://
  • chrome://

and maybe more. I personally think a "file" type icon would be most fitting for most of them. I'm against continuing to use the ⓘ. It feels weird to have it in this situation, it should be removed entirely.

The fallback icon, or the ⓘ icon, doesn't have a tooltip string right now, this would have an accessibility issue here. So, a tooltip for the icon is needed.

See Also: → 1574669

I agree with showing a file icon for file://, resource://, and chrome://.

Do you have any ideas for about: pages? I feel like the info icon might actually make sense for those. Is there any reason not to give them all the chromeUI treatment? I know that the identity popup shows something about "secure Nightly page" for those - is that text inapplicable to the ones that currently don't get the chromeUI treatment?

Flags: needinfo?(jhofmann)

Giving chrome indicators to more about: pages is bug 1567443. I would prefer to not give this out blankly to all about pages, but we can do it with much less overhead by adding a flag to nsIAboutModule. Not a high priority, though. I agree the ⓘ is fine for those.

Flags: needinfo?(jhofmann)

I know Eric was looking at this on UX side.

Flags: needinfo?(epang)

Created a spec that covers the scenarios - please let me know if I'm missing anything.
https://mozilla.invisionapp.com/share/QCUV8DMJTG3#/392611068_Potentially_Secure_Pages

Flags: needinfo?(epang)
Blocks: 1586946

Note that the file icon is placeholder. I'll add an svg of the icon here when ready.

Attached image document-16.svg —

svg for file icon

Thank you!

Priority: P3 → P2

Krystle, would you be interested in working on this? The goal of this bug is to replace the ⓘ icon in the URL bar on file://, resource:// (e.g. resource://gre/modules/Log.jsm) and chrome:// (e.g. chrome://browser/locale/search.properties) pages with the one attached in comment 8.

It's probably a nice learning experience to try and figure out how these icons get assigned using the Browser Toolbox inspect feature, but if you're stuck please let me know and I can tell you what needs changing :)

Flags: needinfo?(krystle.salazar2)

(In reply to Johann Hofmann [:johannh] from comment #10)

Krystle, would you be interested in working on this? The goal of this bug is to replace the ⓘ icon in the URL bar on file://, resource:// (e.g. resource://gre/modules/Log.jsm) and chrome:// (e.g. chrome://browser/locale/search.properties) pages with the one attached in comment 8.

It's probably a nice learning experience to try and figure out how these icons get assigned using the Browser Toolbox inspect feature, but if you're stuck please let me know and I can tell you what needs changing :)

Yes, I'll take a look on it! Please assign it to me.

Flags: needinfo?(krystle.salazar2)
Assignee: nobody → krystle.salazar2
Status: NEW → ASSIGNED

Krystle, did you need any help with this? :)

Flags: needinfo?(krystle.salazar2)
Mentor: jhofmann

Hi Johann, sorry my delay I had complicated the last week, can I have a few more days?

Flags: needinfo?(krystle.salazar2)

Hey, you can absolutely keep working on this, no rush, I was just curious if you were stuck on something. I think we all had some complicated last weeks. :)

Krystle, since you mentioned on Matrix that you would need some help on this, here are the approximate steps to solve it:

  1. Add the new icon to this folder and add it to the jar.inc.mn file. You should be able to view it in your local build using a URL like chrome://global/skin/icons/document.svg now (depending on your file name).
  2. Update this code and rename the "unknownIdentity" bit to something like "localResource" or "localDocument".
  3. Add a new CSS rule that shows your new icon instead of the identity icon, similar to this rule with "localResource" or so instead of "chromeUI"
  4. Update this test to match the updated icon in the correct cases and add potentially missing test URLs with the correct icons.

It might seem like a lot of work but I hope it should actually be relatively straightforward.

Are you still interested in working on this? :)

Flags: needinfo?(krystle.salazar2)

Johann yes! I just pushed the changes following your instructions. Added a test to resource:// URLs too.
Thank you so much for your help! I stay waiting for comments. 😊

Flags: needinfo?(krystle.salazar2)

Rebasing

Depends on D70932

Attachment #9140806 - Attachment is obsolete: true
Attachment #9141450 - Attachment is obsolete: true
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/adac78d7a8f9
Replace (i) icon for a file icon on potentially trustworthy pages. r=johannh

Backed out changeset adac78d7a8f9 (bug 1570678) for siteIdentity related failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=windows%2C10%2Cx64%2Cdebug%2Cmochitests%2Ctest-windows10-64%2Fdebug-mochitest-browser-chrome-e10s-2%2Cm%28bc2%29&fromchange=1ef4c046c26322017c1ef0b454a8ad676e05ad34&tochange=8867863611a0853c2678a6a0f2b72159d2fa6a8d&selectedTaskRun=d3cibzbCR7ubJ8hzDv_D6A-0

Backout link: https://hg.mozilla.org/integration/autoland/rev/8867863611a0853c2678a6a0f2b72159d2fa6a8d

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=301799703&repo=autoland&lineNumber=12013

[task 2020-05-11T21:59:46.355Z] 21:59:46     INFO - TEST-START | browser/base/content/test/siteIdentity/browser_identityBlock_flicker.js
[task 2020-05-11T21:59:46.365Z] 21:59:46     INFO - GECKO(7480) | [Parent 6084: Main Thread]: I/DocShellAndDOMWindowLeak ++DOCSHELL 000001C120026000 == 6 [pid = 6084] [id = {bf7068c3-7c36-4c12-90db-5d32d0530b9a}]
[task 2020-05-11T21:59:46.365Z] 21:59:46     INFO - GECKO(7480) | [Parent 6084: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 17 (000001C125B728C0) [pid = 6084] [serial = 125] [outer = 0000000000000000]
[task 2020-05-11T21:59:46.365Z] 21:59:46     INFO - GECKO(7480) | [Parent 6084: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 18 (000001C120029000) [pid = 6084] [serial = 126] [outer = 000001C125B728C0]
[task 2020-05-11T21:59:46.443Z] 21:59:46     INFO - GECKO(7480) | [Parent 6084, Main Thread] WARNING: NS_ENSURE_TRUE(rootFrame) failed: file /builds/worker/checkouts/gecko/dom/base/nsGlobalWindowOuter.cpp, line 4285
[task 2020-05-11T21:59:46.443Z] 21:59:46     INFO - GECKO(7480) | [Parent 6084: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 19 (000001C122386400) [pid = 6084] [serial = 127] [outer = 000001C125B728C0]
[task 2020-05-11T21:59:46.684Z] 21:59:46     INFO - TEST-INFO | started process screenshot
[task 2020-05-11T21:59:46.763Z] 21:59:46     INFO - TEST-INFO | screenshot: exit 0
[task 2020-05-11T21:59:46.763Z] 21:59:46     INFO - Buffered messages logged at 21:59:46
[task 2020-05-11T21:59:46.763Z] 21:59:46     INFO - Entering test bound test
[task 2020-05-11T21:59:46.763Z] 21:59:46     INFO - Buffered messages finished
[task 2020-05-11T21:59:46.763Z] 21:59:46     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/siteIdentity/browser_identityBlock_flicker.js | identity box has the correct class - Got localResource, expected unknownIdentity
[task 2020-05-11T21:59:46.763Z] 21:59:46     INFO - Stack trace:
[task 2020-05-11T21:59:46.763Z] 21:59:46     INFO - chrome://mochikit/content/browser-test.js:test_is:1327
[task 2020-05-11T21:59:46.763Z] 21:59:46     INFO - chrome://mochitests/content/browser/browser/base/content/test/siteIdentity/browser_identityBlock_flicker.js:test:15
[task 2020-05-11T21:59:46.764Z] 21:59:46     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1064
[task 2020-05-11T21:59:46.764Z] 21:59:46     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1104
[task 2020-05-11T21:59:46.764Z] 21:59:46     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:927
[task 2020-05-11T21:59:46.764Z] 21:59:46     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:918
[task 2020-05-11T21:59:46.764Z] 21:59:46     INFO - GECKO(7480) | [Parent 6084, Main Thread] ###!!! ASSERTION: Wrong event target!: 'this == fl->GetBrowserChildMessageManager()', file /builds/worker/checkouts/gecko/dom/base/InProcessBrowserChildMessageManager.cpp, line 254
[task 2020-05-11T21:59:46.862Z] 21:59:46     INFO - GECKO(7480) | #01: static mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget *>*) [dom/events/EventDispatcher.cpp:907]
[task 2020-05-11T21:59:46.862Z] 21:59:46     INFO - GECKO(7480) | #02: static mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsPresContext*, nsEventStatus*) [dom/events/EventDispatcher.cpp:1164]
[task 2020-05-11T21:59:46.863Z] 21:59:46     INFO - GECKO(7480) | #03: mozilla::DOMEventTargetHelper::DispatchEvent(mozilla::dom::Event&, mozilla::dom::CallerType, mozilla::ErrorResult&) [dom/events/DOMEventTargetHelper.cpp:169]
[task 2020-05-11T21:59:46.863Z] 21:59:46     INFO - GECKO(7480) | #04: mozilla::dom::EventTarget::DispatchEvent(mozilla::dom::Event&, mozilla::ErrorResult&) [dom/events/EventTarget.cpp:184]
[task 2020-05-11T21:59:46.863Z] 21:59:46     INFO - GECKO(7480) | #05: mozilla::DOMEventTargetHelper::DispatchTrustedEvent(nsTSubstring<char16_t> const&) [dom/events/DOMEventTargetHelper.cpp:183]
[task 2020-05-11T21:59:46.863Z] 21:59:46     INFO - GECKO(7480) | #06: mozilla::dom::InProcessBrowserChildMessageManager::FireUnloadEvent() [dom/base/InProcessBrowserChildMessageManager.cpp:215]
[task 2020-05-11T21:59:46.864Z] 21:59:46     INFO - GECKO(7480) | #07: nsFrameLoader::DestroyDocShell() [dom/base/nsFrameLoader.cpp:1918]
[task 2020-05-11T21:59:46.864Z] 21:59:46     INFO - GECKO(7480) | #08: nsFrameLoaderDestroyRunnable::Run() [dom/base/nsFrameLoader.cpp:1868]
[task 2020-05-11T21:59:46.864Z] 21:59:46     INFO - GECKO(7480) | #09: mozilla::dom::Document::MaybeInitializeFinalizeFrameLoaders() [dom/base/Document.cpp:8498]
[task 2020-05-11T21:59:46.864Z] 21:59:46     INFO - GECKO(7480) | #10: mozilla::detail::RunnableMethodImpl<mozilla::dom::Document *,void (mozilla::dom::Document::*)(),1,mozilla::RunnableKind::Standard>::Run() [xpcom/threads/nsThreadUtils.h:1239]
[task 2020-05-11T21:59:46.865Z] 21:59:46     INFO - GECKO(7480) | #11: static nsContentUtils::RemoveScriptBlocker() [dom/base/nsContentUtils.cpp:5345]
[task 2020-05-11T21:59:46.865Z] 21:59:46     INFO - GECKO(7480) | #12: nsFrameLoaderOwner::ChangeRemotenessCommon(nsFrameLoaderOwner::ChangeRemotenessContextType const&, bool, nsTSubstring<char16_t> const&, std::function<void ()>&, mozilla::ErrorResult&) [dom/base/nsFrameLoaderOwner.cpp:142]
[task 2020-05-11T21:59:46.865Z] 21:59:46     INFO - GECKO(7480) | #13: nsFrameLoaderOwner::ChangeRemoteness(mozilla::dom::RemotenessOptions const&, mozilla::ErrorResult&) [dom/base/nsFrameLoaderOwner.cpp:200]
[task 2020-05-11T21:59:46.865Z] 21:59:46     INFO - GECKO(7480) | #14: mozilla::dom::XULFrameElement_Binding::changeRemoteness(JSContext*, JS::Handle<JSObject *>, void*, JSJitMethodCallArgs const&) [s3:gecko-generated-sources:1b735a5827115d5d2069243527a6e3772d0c4d6ff4bdaf3f63ab8de5dc996b2fcb0cca3da1017178efe66c3c839c295239f63675eef6d3079b565db0f7be4443/dom/bindings/XULFrameElementBinding.cpp::385]
[task 2020-05-11T21:59:46.865Z] 21:59:46     INFO - GECKO(7480) | #15: mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy,mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) [dom/bindings/BindingUtils.cpp:3205]
[task 2020-05-11T21:59:46.865Z] 21:59:46     INFO - GECKO(7480) | #16: CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) [js/src/vm/Interpreter.cpp:493]
[task 2020-05-11T21:59:46.866Z] 21:59:46     INFO - GECKO(7480) | #17: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) [js/src/vm/Interpreter.cpp:585]
[task 2020-05-11T21:59:46.866Z] 21:59:46     INFO - GECKO(7480) | #18: InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) [js/src/vm/Interpreter.cpp:648]
[task 2020-05-11T21:59:46.866Z] 21:59:46     INFO - GECKO(7480) | #19: Interpret(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:3312]
[task 2020-05-11T21:59:46.866Z] 21:59:46     INFO - GECKO(7480) | #20: js::RunScript(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:465]
[task 2020-05-11T21:59:46.867Z] 21:59:46     INFO - GECKO(7480) | #21: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) [js/src/vm/Interpreter.cpp:620]
[task 2020-05-11T21:59:46.867Z] 21:59:46     INFO - GECKO(7480) | #22: InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) [js/src/vm/Interpreter.cpp:648]
[task 2020-05-11T21:59:46.867Z] 21:59:46     INFO - GECKO(7480) | #23: js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) [js/src/vm/Interpreter.cpp:665]
[task 2020-05-11T21:59:46.868Z] 21:59:46     INFO - GECKO(7480) | #24: js::CallSelfHostedFunction(JSContext*, JS::Handle<js::PropertyName *>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) [js/src/vm/SelfHosting.cpp:1673]
[task 2020-05-11T21:59:46.868Z] 21:59:46     INFO - GECKO(7480) | #25: js::jit::InterpretResume(JSContext*, JS::Handle<JSObject *>, JS::Value*, JS::MutableHandle<JS::Value>) [js/src/jit/VMFunctions.cpp:995]
[task 2020-05-11T21:59:46.868Z] 21:59:46     INFO - GECKO(7480) | #26: ??? (???:???)
[task 2020-05-11T21:59:46.869Z] 21:59:46     INFO - GECKO(7480) | [Parent 6084, Main Thread] ###!!! ASSERTION: Wrong message manager!: 'fl->mMessageManager == mChromeMessageManager', file /builds/worker/checkouts/gecko/dom/base/InProcessBrowserChildMessageManager.cpp, line 256
[task 2020-05-11T21:59:46.869Z] 21:59:46     INFO - GECKO(7480) | #01: static mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget *>*) [dom/events/EventDispatcher.cpp:907]
[task 2020-05-11T21:59:46.869Z] 21:59:46     INFO - GECKO(7480) | #02: static mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsPresContext*, nsEventStatus*) [dom/events/EventDispatcher.cpp:1164]
[task 2020-05-11T21:59:46.869Z] 21:59:46     INFO - GECKO(7480) | #03: mozilla::DOMEventTargetHelper::DispatchEvent(mozilla::dom::Event&, mozilla::dom::CallerType, mozilla::ErrorResult&) [dom/events/DOMEventTargetHelper.cpp:169]
[task 2020-05-11T21:59:46.869Z] 21:59:46     INFO - GECKO(7480) | #04: mozilla::dom::EventTarget::DispatchEvent(mozilla::dom::Event&, mozilla::ErrorResult&) [dom/events/EventTarget.cpp:184]
[task 2020-05-11T21:59:46.870Z] 21:59:46     INFO - GECKO(7480) | #05: mozilla::DOMEventTargetHelper::DispatchTrustedEvent(nsTSubstring<char16_t> const&) [dom/events/DOMEventTargetHelper.cpp:183]
[task 2020-05-11T21:59:46.870Z] 21:59:46     INFO - GECKO(7480) | #06: mozilla::dom::InProcessBrowserChildMessageManager::FireUnloadEvent() [dom/base/InProcessBrowserChildMessageManager.cpp:215]
[task 2020-05-11T21:59:46.872Z] 21:59:46     INFO - GECKO(7480) | #07: nsFrameLoader::DestroyDocShell() [dom/base/nsFrameLoader.cpp:1918]
[task 2020-05-11T21:59:46.872Z] 21:59:46     INFO - GECKO(7480) | #08: nsFrameLoaderDestroyRunnable::Run() [dom/base/nsFrameLoader.cpp:1868]
[task 2020-05-11T21:59:46.872Z] 21:59:46     INFO - GECKO(7480) | #09: mozilla::dom::Document::MaybeInitializeFinalizeFrameLoaders() [dom/base/Document.cpp:8498]
[task 2020-05-11T21:59:46.873Z] 21:59:46     INFO - GECKO(7480) | #10: mozilla::detail::RunnableMethodImpl<mozilla::dom::Document *,void (mozilla::dom::Document::*)(),1,mozilla::RunnableKind::Standard>::Run() [xpcom/threads/nsThreadUtils.h:1239]
[task 2020-05-11T21:59:46.873Z] 21:59:46     INFO - GECKO(7480) | #11: static nsContentUtils::RemoveScriptBlocker() [dom/base/nsContentUtils.cpp:5345]
...
...
Flags: needinfo?(krystle.salazar2)

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:krystle.salazar2, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(krystle.salazar2)
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d954ca3baaf5
Replace (i) icon for a file icon on potentially trustworthy pages. r=johannh,nhnt11

Please also check failures on browser_deprecatedTLSVersions.js
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=304762001&repo=autoland&lineNumber=10947

Hrmm this landed in the wrong order with bug 1496844....

Flags: needinfo?(krystle.salazar2)
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6024bb071713
Replace (i) icon for a file icon on potentially trustworthy pages. r=johannh,nhnt11

https://hg.mozilla.org/mozilla-central/rev/6024bb071713

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox79: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 79
See Also: → 1644783
See Also: → 1647653
Regressions: 1647846
Regressions: 1651062
Regressions: 1656027
Regressions: 1667965
Regressions: 1746383
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: