Network monitor toolbar is 1px too high

RESOLVED FIXED in Firefox 67

Status

defect
P3
normal
RESOLVED FIXED
10 months ago
4 months ago

People

(Reporter: fvsch, Assigned: lloanalas, Mentored, NeedInfo)

Tracking

({good-first-bug})

unspecified
Firefox 67

Firefox Tracking Flags

(firefox67 fixed)

Details

(Whiteboard: good-first-bug,)

Attachments

(4 attachments)

Reporter

Description

10 months ago
For consistency with other parts of DevTools, the Network monitor toolbar (or toolbars, when splitted in 2) should be 28px high excluding borders.

Currently it’s 29px, 1px taller than the toolbox's tab bar, and 1px taller than the Console’s toolbar.

The issue comes from this code:

.devtools-toolbar-two-rows-1,
.devtools-toolbar-two-rows-2,
.devtools-toolbar-single-row {
  flex-grow: 1;
  min-height: var(--primary-toolbar-height);
  background-color: var(--theme-body-background);
}

The --primary-toolbar-height variable is set to 29px, which is intended for a toolbar which has a 1px border-bottom, leaving 28px for the content/padding area.
Here, only the .devtools-toolbar-two-rows-2 has a bottom border.

One solution is to give a border-bottom to each toolbar, and remove the border on `.devtools-toolbar-container`.

It might also be better to remove the `padding: 0 3px` on that container, since it makes for an incomplete border (probably on purpose, but not consistent with other tools which have full-width borders), and it leaves 3px of darker gray on each side.

I can probably make a patch.
Reporter

Comment 1

10 months ago
It looks like this toolbar has a different background from most other toolbars. It uses the `--theme-body-background`, which is white in the light theme and Grey-80 (#2a2a2e) in the dark theme.

I would like to use the standard toolbar background instead (shown here in the light theme). It would be more consistent with the Inspector, Console and Storage tools, for instance.

Victoria, would that look like a good change to you?
Attachment #9005176 - Flags: ui-review?(victoria)
Comment on attachment 9005176 [details]
network-toolbar-background.png

The white search bar/toolbar in light-mode Network is actually intended :) — there's a bug somewhere to change the Console to match it, and the Inspector will be getting this style of white rectangular search bar as well. It's a way to set this style of toolbar, with large flexible search input, apart from other types of toolbars, such as the tab bars.

I realize using Gray 80 for this is a bit inconsistent with the usual dark-to-light vs light-to-dark patterns for light/dark mode bars, but I feel it looks right in this case.
Attachment #9005176 - Flags: ui-review?(victoria) → ui-review-
So, the background should stay as is, but we should fix the height.
Good instructions are already in comment #0

Honza
Mentor: odvarko
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: good-first-bug
Priority: -- → P3
Whiteboard: good-first-bug,

Comment 4

9 months ago
Hi is this assigned? I would like to pick this up if someone can guide me.. this is my first time contributing
(In reply to shobhit.kr from comment #4)
> Hi is this assigned? I would like to pick this up if someone can guide me..
> this is my first time contributing

Assigned to you!

You can start with this MDN page:
https://developer.mozilla.org/en-US/docs/Tools/Contributing

Honza
Assignee: nobody → shobhit.kr
Status: NEW → ASSIGNED
@shobhit: How is it going, do you need any help?

Honza
Flags: needinfo?(shobhit.kr)

Comment 7

9 months ago
Hello! I'm looking to be assigned to a good first bug as well. Is @shobhit still assigned? Could I be assigned in case it becomes available?
(In reply to Deepashree from comment #7)
> Hello! I'm looking to be assigned to a good first bug as well. Is @shobhit
> still assigned? Could I be assigned in case it becomes available?
Yes, but let's give shobhit a few days to reply.
I no reply in 2 days, ping me I'll reassign it.

Honza

Comment 9

9 months ago
(In reply to Jan Honza Odvarko [:Honza] from comment #6)
> @shobhit: How is it going, do you need any help?
> 
> Honza

Hi -- sorry I got preoccupied with some personal work
I tried building firefox but I am stuck
I am trying to do it on Linux Mint 18.3 and this comes up "Only GCC 6.1 or newer is supported (found version 5.4.0)"
Running "gcc-6 --version" gives me this "gcc-6 (Ubuntu 6.4.0-17ubuntu1~16.04) 6.4.0 20180424"

I could see a related bug opened as well https://bugzilla.mozilla.org/show_bug.cgi?id=1444274
not sure if this is the same problem I am having?
Flags: needinfo?(shobhit.kr)
Reporter

Comment 10

9 months ago
Regarding the GCC issues: for working on CSS, you might not need to use gcc at all.
Did you run the `./mach bootstrap` command when getting started? If yes, did you pick the option to build Firefox for desktop with artifact builds?

Artifact builds use precompiled binaries and inject your CSS and JS changes. There should be no gcc involved. (But maybe the bootstrap command does try to check your gcc version in any case, so I can't guarantee it'll work.)

Comment 11

9 months ago
(In reply to Florens Verschelde from comment #10)
> Regarding the GCC issues: for working on CSS, you might not need to use gcc
> at all.
> Did you run the `./mach bootstrap` command when getting started? If yes, did
> you pick the option to build Firefox for desktop with artifact builds?
> 
> Artifact builds use precompiled binaries and inject your CSS and JS changes.
> There should be no gcc involved. (But maybe the bootstrap command does try
> to check your gcc version in any case, so I can't guarantee it'll work.)

Yeah, I got it working, I guess some files did not download properly. Now when I am trying to build I get this message
"gecko-dev/obj-x86_64-pc-linux-gnu/dist/system_wrappers/cstddef:3:15: fatal error: 'cstddef' file not found"

All the dependencies seemed fine when I ran mach build.. I am not able to launch firefox without having this build completed

Comment 12

4 months ago

Hello, I'm looking to be assigned to a good first bug. Could I assigned, if its available?

(In reply to Shovan Chatterjee from comment #12)

Hello, I'm looking to be assigned to a good first bug. Could I assigned, if its available?

Done, thanks for the help!

Honza

Assignee: shobhit.kr → shovanc.web
Assignee

Comment 14

4 months ago

Hi,

I'm an outreachy applicant. If this bug is still available, I'd be happy try working on it. Haven't seen any activity so figured I'd show my interest in working on it.

Thanks a ton!

Flags: needinfo?(odvarko)

@Shovan: are you working on this?

Honza

Flags: needinfo?(shovanc.web)

(In reply to lloan:[lloan] from comment #14)

I'm an outreachy applicant. If this bug is still available, I'd be happy try working on it. Haven't seen any activity so figured I'd show my interest in working on it.

Assigned to you, thanks!
Honza

Assignee: shovanc.web → lloanalas
Flags: needinfo?(odvarko)
Assignee

Comment 17

4 months ago
Posted image Test for Bug 1486323

I modified a bit more than the original suggestion pointed out. Even after doing what comment #0 suggested there was still a remainder of 0.8px on one of the rows. It seems that the search element height is static and set to 29px - went ahead and changed it to 28px as it also has a border. It still functions the same way and looks the same way as it did before and now the rows are the same size.

Flags: needinfo?(odvarko)
Assignee

Comment 18

4 months ago

Bug 1486323 - Network monitor toolbar is 1px too high. r=honza

Thanks for working on this!
Honza

Flags: needinfo?(odvarko)

Comment 20

4 months ago
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f2bd560573a4
Network monitor toolbar is 1px too high. r=Honza

Comment 21

4 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
Assignee

Comment 22

4 months ago

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #19)

Thanks for working on this!
Honza

Awesome - thanks a ton for the help :)!

You need to log in before you can comment on or make changes to this bug.