1496440 - Netmonitor - cancel button to filter url not centered

Status: RESOLVED WORKSFORME

(DevTools :: Netmonitor, defect, P3)

Description

[Cristian Fogel [:cfogel]]

[Affected versions]:
- 64.0a1 (2018-10-02), 63.0b11, 62.0.3

[Affected platforms]:
- win10x64, macOS 10.9, Ubuntu16.04

[Steps to reproduce]:
1. Launch firefox, open DevTools;
2. Click on the Network(monitor) tab;
3. Click inside the filter url bar;
4. Type any text;

[Expected result]:
- the (x) button is properly displayed at the right-corner of the input field

[Actual result]:
- the (x) button should either be centered or have it's size increased

[Regression range]:
- introduced with bug 1456430

[Additional notes]:
- attached screenshot with the issue

Comment 1

[Cristian Fogel [:cfogel]]

Attachment: network_cancelX_notCentered.png

Above: old version;
Bellow: post the focus ring adjustments.

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks for the report!

Honza

Comment 3

[Reshma]

Hello,
Can this be assigned to me?

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Reshma from comment #3)
> Hello,
> Can this be assigned to me?
Yes, done, thanks for the help!
Honza

Comment 5

[Reshma]

Hey @honza

 So the cancel icon has the class .devtools-searchinput-clear,[1] whose position is aligned with respect to the searchbox with classname .devtools-searchbox,[2] 
 The .devtools-searchbox has height 23px which is rewritten in the netmonitor searchbox to 29px which cause the cancel icon to not be centered.[3]

So I still haven’t figured out if, the .devtools-toolbar-group needs the 29px height? If so, then we should remove .devtools-searchbox from this line.[3]

What do you think?

[1]https://searchfox.org/mozilla-central/source/devtools/client/themes/common.css#614
[2]https://searchfox.org/mozilla-central/source/devtools/client/themes/common.css#528
[3]https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/assets/styles/Toolbar.css#121

thanks

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: image.png

Hi Reshma,
I just fetched the latest m-c repo, build Firefox and I don't see the problem any more (Win10) - see the attached screenshot.

Is it only me?
Can you still reproduce the problem on your machine?

Honza

Comment 7

[Reshma]

Attachment: test1.png

hey @honza,

So I am able to reproduce the problem on my machine(win 10)
the problem do seem subtle on my machine see [test1.png] (slight padding on the bottom)compared to the original reporters image.

and in [test2.png] , I used the browser toolbox to change the height to 23px or remove it by doing that the cancel icon seems to be aligned properly.

Comment 8

[Reshma]

Attachment: test2.png

Comment 9

[Reshma]

(In reply to Jan Honza Odvarko [:Honza] from comment #6)
> Created attachment 9017793 [details]
> image.png
> 
> Hi Reshma,
> I just fetched the latest m-c repo, build Firefox and I don't see the
> problem any more (Win10) - see the attached screenshot.
I have fetched the repo but have not build the firefox on my machine yet. I think I didn't fully understand this statement and gave you a reply based on my "testing" done using browser tools.Sorry about that.How about I do this step and update you after that?
> 
> Is it only me?
> Can you still reproduce the problem on your machine?
> 
> Honza

Comment 10

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Reshma from comment #9)
> (In reply to Jan Honza Odvarko [:Honza] from comment #6)
> > Created attachment 9017793 [details]
> > image.png
> > 
> > Hi Reshma,
> > I just fetched the latest m-c repo, build Firefox and I don't see the
> > problem any more (Win10) - see the attached screenshot.
> I have fetched the repo but have not build the firefox on my machine yet. I
> think I didn't fully understand this statement and gave you a reply based on
> my "testing" done using browser tools.Sorry about that.How about I do this
> step and update you after that?
Yes, thanks!

(In reply to Reshma from comment #8)
> Created attachment 9017799 [details]
> test2.png
This screenshot shows that the icons is cut a bit on the right side. Perhaps we could fix that too (bigger padding?)
Let's see after you build Firefox 

Not that you should use:

# Automatically download and use compiled C++ components:
ac_add_options --enable-artifact-builds

...in your .mozconfig file (to speed up the build time)

Honza

Comment 11

[Reshma]

(In reply to Jan Honza Odvarko [:Honza] from comment #10)
> (In reply to Reshma from comment #9)
> > (In reply to Jan Honza Odvarko [:Honza] from comment #6)
> > > Created attachment 9017793 [details]
> > > image.png
> > > 
> > > Hi Reshma,
> > > I just fetched the latest m-c repo, build Firefox and I don't see the
> > > problem any more (Win10) - see the attached screenshot.
> > I have fetched the repo but have not build the firefox on my machine yet. I
> > think I didn't fully understand this statement and gave you a reply based on
> > my "testing" done using browser tools.Sorry about that.How about I do this
> > step and update you after that?
> Yes, thanks!
> 
> (In reply to Reshma from comment #8)
> > Created attachment 9017799 [details]
> > test2.png
> This screenshot shows that the icons is cut a bit on the right side. Perhaps
> we could fix that too (bigger padding?)
Yes, I noticed it too.
> Let's see after you build Firefox 
yes, thanks 
> 
> Not that you should use:
> 
> # Automatically download and use compiled C++ components:
> ac_add_options --enable-artifact-builds
> 
> ...in your .mozconfig file (to speed up the build time)
super confused about this statement.I will search more about this.
I am referencing to this-https://developer.mozilla.org/en-US/docs/Tools/Contributing link which you gave me. 

Is there any other links to refer and learn more about what you said? 

thanks
> 
> Honza

Comment 12

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Read more about Aftifact builds here:

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds

Honza

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

Marking fix-optional for 64. We could still take a patch for 65, and if it's verified and doesn't seem risky, could still take fixes for 64 as well.

Comment 14

[Jan Honza Odvarko [:Honza] (always need-info? me)]

@Reshma: Are you still working on this? Is there anything I can help with?

Honza

Comment 15

[Reshma]

(In reply to Jan Honza Odvarko [:Honza] (need-info? me) from comment #14)
> @Reshma: Are you still working on this? Is there anything I can help with?
> 
> Honza
[1]While building using https://developer.mozilla.org/en-US/docs/Tools/Contributing.
when I run ./mach bootstrap
--------------------------------------------------------------------------------------------------------------------------------
Error running mach:

    ['bootstrap']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.

You should consider filing a bug for this issue.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

NotImplementedError: Bootstrap support for Windows is under development. For now use MozillaBuild to set up a build environment on Windows. If you are testing Windows Bootstrap support, try `export MOZ_WINDOWS_BOOTSTRAP=1`

  File "D:gecko-dev\python/mozboot/mozboot/mach_commands.py", line 37, in bootstrap
    no_system_changes=no_system_changes)
  File "D:gecko-dev\python/mozboot\mozboot\bootstrap.py", line 251, in __init__
    self.instance = cls(**args)
  File "D:gecko-dev\python/mozboot\mozboot\windows.py", line 43, in __init__
    raise NotImplementedError('Bootstrap support for Windows is under development. For '

--------------------------------------------------------------------------------------------------------------------------------
[2]Before this, due to python3 I got errors for runnig mach, then through docs came to know I need python 2.7
After installing that I got errors for ./mach build same as ./mach bootstrap

[3]I searched for and still figuring solutions but can't seem to make it work.l Looked for bugs in Bugzilla got to know [bug 1418900] similar to mine. There is also guide for windows that on mdn that says to install rust , but if not run mach bootstrap.

[4] I created  mozconfig file and included what you suggested earlier:
--------------------------------------------------------
# Automatically download and use compiled C++ components:
ac_add_options --enable-artifact-builds
---------------------------------------------------------
[5] And according to https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds
article if i include :
------------------------------------------------------------------------------------------------------------------------------
# Enable debug versions of the pre-built binary artifacts:
ac_add_options --enable-debug

# Automatically download and use compiled C++ components:
ac_add_options --enable-artifact-builds

# Write build artifacts to:
mk_add_options MOZ_OBJDIR=./objdir-frontend-debug-artifact
------------------------------------------------------------------------------------------------------------------------------
I got errors for pointing my mozconfig file so I deleted those to only include artifact-builds

I just can't seem to figure out what mistakes am I doing.
I hope I am not bothering too much with my doubts, this is getting very complicated for me.
I liked working with browser toolbox more.

Comment 16

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Did you install MozillaBuild package on your machine?

https://wiki.mozilla.org/MozillaBuild

Honza

Comment 17

[Reshma]

(In reply to Jan Honza Odvarko [:Honza] (need-info? me) from comment #16)
> Did you install MozillaBuild package on your machine?
> 
> https://wiki.mozilla.org/MozillaBuild
> 
> Honza

I did not install that before.

I installed it now in C:\

and tried same steps of .\mach bootstrap in gecko-dev which is in D:\
should that be in C drive instead?

I am getting the same results I mentioned earlier.
NotImplementedError: Bootstrap support for Windows is under development.
For now, use MozillaBuild to set up a build environment on Windows. 
does the above line mean I should-- hg clone https://hg.mozilla.org/mozilla-central
and then do the ./mach build?

Also, why do I get the following, immediately after I run ./mach bootstrap

$ ./mach bootstrap
which: no python2.7 in (/c/Users/reshma/bin:/mingw64/bin:/usr/local/bin:/usr/bin:/bin:/mingw64/bin:/usr/bin:/c/Users/reshma/bin:/c/Python27/Scripts:/c/Python27:------------bunch of paths...)

I clearly have too many questions and errors, So should I be asking all this in slack or anywhere particularly?

Thanks @honza

Comment 18

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Reshma from comment #17)
> and tried same steps of .\mach bootstrap in gecko-dev which is in D:\
> should that be in C drive instead?
> 
> I am getting the same results I mentioned earlier.
You need to run .\mach bootstrap in this shell:
D:\mozilla-build\start-shell.bat

> I clearly have too many questions and errors, So should I be asking all this
> in slack or anywhere particularly?
Yes, mozilla.slack.com, #firefox channel should be good place to ask these questions.

Honza

Comment 19

[Liz Henry (:lizzard) (relman/hg->git project)]

Happy to take a patch in nightly; if it seems low risk enough please feel free to request uplift to 65 beta.

Comment 20

[Hengyeow (:tanhengyeow)]

Hi Honza

This bug doesn't seem to be applicable anymore when I tested it recently. Could you verify it again? Thanks :)

Also I followed the conversation and understand there might be some UX issues to fix, should we shift that to a different bug?

Comment 21

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Heng Yeow (:tanhengyeow) from comment #20)

This bug doesn't seem to be applicable anymore when I tested it recently. Could you verify it again? Thanks :)
I can't reproduce that either, closing.

Also I followed the conversation and understand there might be some UX issues to fix, should we shift that to a different bug?
Not sure what you mean. The icon was cut a bit on the right side, but I can't reproduce that either.
But if you see something than yes we should create a new report.

Thanks!
Honza