Localhost security state icon doesn't inherit text color

RESOLVED FIXED in Firefox 67

Status

enhancement
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: fvsch, Assigned: fvsch)

Tracking

unspecified
Firefox 67

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(5 attachments)

Assignee

Description

5 months ago

It's black on dark blue for selected rows, black on dark gray in the dark theme.

The globe icon we're using is also a bit massive in that list. We could go for a small variant (12px instead of 16px).

Assignee

Updated

5 months ago
Assignee: nobody → florens
Status: NEW → ASSIGNED
Assignee

Comment 1

5 months ago

Example using currentColor and a 12px icon, which is closer in size to the padlock icons we use for secure and insecure connections.

Posted image image.png

Thanks for the patch Florens!

I noticed that the globe and domain label are not centered at the same line (and neither are the other columns). What about addding the following two CSS props yet:

diff --git a/devtools/client/netmonitor/src/assets/styles/RequestList.css b/devtools/client/netmonitor/src/assets/styles/RequestList.css
--- a/devtools/client/netmonitor/src/assets/styles/RequestList.css
+++ b/devtools/client/netmonitor/src/assets/styles/RequestList.css
@@ -73,17 +73,16 @@
 }

 .requests-list-column {
   display: table-cell;
   cursor: default;
   white-space: nowrap;
   overflow: hidden;
   text-overflow: ellipsis;
-  vertical-align: middle;
   max-width: 50px;
   min-width: 50px;
 }

 .requests-list-column > * {
   display: inline-block;
 }

@@ -343,17 +342,17 @@
   text-align: start;
 }

 .requests-security-state-icon {
   display: inline-block;
   width: 16px;
   height: 16px;
   margin: 0 4px;
-  vertical-align: text-bottom;
+  vertical-align: text-top;
   background-position: center;
   background-repeat: no-repeat;
   -moz-context-properties: fill;
   fill: currentColor;
 }

 .request-list-item.selected .requests-security-state-icon {
   filter: brightness(1.3);

This seems to fix the Domain columns as well as the icon in Status column.
(Tested on Win10)

Honza

Flags: needinfo?(florens)
Assignee

Comment 4

4 months ago

This doesn't seem to work well for me on Linux. (Haven't tried macOS.)

Currently I'm getting row cells that are 24.5px tall (no idea why .5 and not 24px, given the line-height:24px). And the icons look correctly aligned with the current styles. With your suggested changes, they appear a bit too low.

I can get some decent vertical alignment with:

  • removing the 24px line-height
  • adding padding: 4px 0; to the cells
  • wrapping every bit of text in a <span> with display:inline-block;vertical-align:middle;line-height:16px;
  • and vertical-align:middle for the images

Note that some images may not be perfectly visually centered in their canvas (e.g. if the SVG paths are 13px tall in a 16px canvas), which may make some things look a bit off.

Flags: needinfo?(florens)

(In reply to Florens Verschelde :fvsch from comment #4)

Created attachment 9041803 [details]
netmonitor-icons-textbottom.png

This doesn't seem to work well for me on Linux. (Haven't tried macOS.)

OK, I see, thanks for testing. I think that the actual patch is ready enough and we can land it.
I just accepted it in Phabricator.

Thanks Florens!
Honza

Comment 6

4 months ago
Pushed by florens@fvsch.com:
https://hg.mozilla.org/integration/autoland/rev/9611438fd389
Make the netmonitor localhost icon use a smaller size and inherit currentColor; r=Honza

Comment 7

4 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
You need to log in before you can comment on or make changes to this bug.