1409651 - Security side panel has different color

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P2)

Description

[[e-mail]]

Attachment: security panel

Security side panel has different color set than other panels. Should apply same photon color as other side panels (title in deep blue color + description in red/magenta color)

Could use inspector in Browser Toolbox (menu `Web Developer > Browser Toolbox`) to check the exact color code.


As the result, should add `color: var(--string-color);` style for security panel's description/value fields.

Comment 1

[Pradeep Gangwar [:pradeepgangwar]]

I would be happy to work on this bug. Please assign it to me.

Comment 2

[[e-mail]]

Great! Let me know if you need any help.

Comment 3

[Pradeep Gangwar [:pradeepgangwar]]

:gasolin Sir, I am facing some trouble in running netmonitor in browser mode. It displays everything correctly. But on clicking reload button it does nothing. It worked for one or two times perfectly. I have posted some screenshots in #netmonitor channel in slack. Please help if it is a known issue.

Comment 4

[Pradeep Gangwar [:pradeepgangwar]]

Cancelling needinfo as i got requested information about problem from slack. :)

Comment 5

[Pradeep Gangwar [:pradeepgangwar]]

Attachment: Screenshot-2017-10-30 Netmonitor.png

Comment 6

[Pradeep Gangwar [:pradeepgangwar]]

Attachment: Screenshot-2017-10-18 Netmonitor(2).png

Sir, This is how it looks on applying patch. However i haven't pushed to MozReview because it was not working for me. I tried to get help from IRC and slack but there was no clue. I will wait for some more time else will attache patch here.

Comment 7

[[e-mail]]

Looks good! Provide .patch or .diff also works

For you reference, here's how I setup my environment
https://blog.gasolin.idv.tw/2016/08/08/The-newbies-workflow-on-Mozilla-Gecko-project/

Comment 8

[Pradeep Gangwar [:pradeepgangwar]]

Attachment: Bug 1409651.patch

Comment 9

[[e-mail]]

Comment on attachment 8923826 [details] [diff] [review]
Bug 1409651.patch

I think we sould not modify treeview class with external `--string-color` vairable since it brings extranal dependency for treeview component.

Please try to apply the css within security panels.

Comment 10

[[e-mail]]

Note the security panel's css style place will be changed to `NetworkDetailsPanel.css` after bug 1409413, please keep that bug in mind.

Comment 11

[Pradeep Gangwar [:pradeepgangwar]]

Should i wait for the bug 1409413 to be resolved or i can continue fixing this.

Comment 12

[[e-mail]]

I suggest waiting bug 1409413 lands. Or you can apply that patch locally and continue to fix this.

Comment 13

[Pradeep Gangwar [:pradeepgangwar]]

I push my commits to review through `hg push review`. It landed my patch to https://reviewboard-hg.mozilla.org/gecko/rev/3f4b3d813f1b and my terminal didn't proceed to show any other output neither changes were displayed here. Please review this.

Comment 14

[[e-mail]]

I use git cinnabar so I'm not sure what happens as well.

Ricky, do you know why `hg push review` not send the patch to bugzilla?

Comment 15

[Ricky Chien [:rickychien] (inactive)]

I can diagnose your problem because there is no output in your terminal. Could you please try to reset your Mozreview according to doc [1]? It doesn't make sense that there is no message output from console. Otherwise, I think Splinter Review (the current way you're using) is also an adequate alternative for code review if you are still unable to setup mozreview properly.

[1] http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/install.html

Comment 16

[Ricky Chien [:rickychien] (inactive)]

s/I can/I can't/

Comment 17

[Pradeep Gangwar [:pradeepgangwar]]

Attachment: Bug 1409651- Fix security side panel color in netmonitor;

Review commit: https://reviewboard.mozilla.org/r/198636/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/198636/

Comment 18

[[e-mail]]

Comment on attachment 8927309 [details]
Bug 1409651- Fix security side panel color in netmonitor;

https://reviewboard.mozilla.org/r/198636/#review204002

The change looks good to me,  Thanks!
We can do a little enhancement for the style consistency & readability before land.

::: devtools/client/netmonitor/src/assets/styles/NetworkDetailsPanel.css:134
(Diff revision 1)
>  .network-monitor .tree-container .treeTable .treeRow.tree-section > .treeLabelCell > .treeLabel:hover,
>  .network-monitor .tree-container .treeTable .treeRow.tree-section > .treeValueCell:not(:hover) * {
>    color: var(--theme-toolbar-color);
>  }
>  
> +.network-monitor .treeTable .treeValueCell input{

nit: for consistency with other styles, 

We can use `.network-monitor .tree-container .treeTable .treeValueCell input{`

and please put this after `.network-monitor .tree-container .treeTable .treeValueCell {` style

Comment 19

[Pradeep Gangwar [:pradeepgangwar]]

Comment on attachment 8927309 [details]
Bug 1409651- Fix security side panel color in netmonitor;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/198636/diff/1-2/

Comment 20

[[e-mail]]

Comment on attachment 8927309 [details]
Bug 1409651- Fix security side panel color in netmonitor;

https://reviewboard.mozilla.org/r/198636/#review204036

thanks for the update!

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/055409746927
Fix security side panel color in netmonitor; r=gasolin

Comment 22

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/055409746927

Comment 23

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 53.0a1 (2017-10-18)  in Ubuntu 12.04 LTS!

This bug's fix is verified with latest Nightly!

Build ID   : 20171114220116
User Agent : Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101 Firefox/59.0

Comment 24

[Sajedul Islam]

I have reproduced this bug with Nightly 58.0a1 (2017-10-18) on Windows 8.1 , 64 Bit ! 

This bug's fix is Verified with latest Nightly!

Build   ID    20171227100314
User Agent    Mozilla/5.0 (Windows NT 6.3; WOW64; rv:59.0) Gecko/20100101 Firefox/59.0

[bugday-20171227]