1425839 - Splitter does not have a border at the boundary of requests list header and details tab

Status: VERIFIED FIXED

(DevTools :: Netmonitor, defect, P3)

Description

[magicp]

Attachment: splitter-border.png

Steps to reproduce:
1. Launch Nightly
2. Open Netmonitor (Ctrl+Shift+E)
3. Go to any sites (e.g. https://bugzilla.mozilla.org)
4. Set "CSS" filter because prevent showing vertical scrollbar in requests list
5. Select any requests > Details panel will be opened

Actual results:
Splitter does not have a border at the boundary of requests list header and details tab.

Expected results:
Splitter should have a border.

Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=10777aa50c49e929145fcd84a10892c0db099862&tochange=083a5838f76a418779c2f4fc01152bc3be355fc0

Comment 1

[tera_1225]

Hi, I've made a quick patch for this. Not sure how it ended up getting inserted with my patch but this should fix it. I'll push to Mozreview if the tests come back ok.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7e1dd42d27a8778efbaeb020450b56ebc4d2a86

Comment 2

[tera_1225]

Attachment: Bug 1425839 - Added right border to.requests-list-headers

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

Comment 3

[tera_1225]

Attachment: Bug 1425839 Reduced requests-list-headers-wrapper z-index from 1000 to 1.

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

Comment 4

[Ricky Chien [:rickychien] (inactive)]

The root cause of this is due to `z-index: 1000` at https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/assets/styles/RequestList.css#103

I'd prefer to remove this since it's useless anymore.

Comment 5

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8937855 [details]
Bug 1425839 Reduced requests-list-headers-wrapper z-index from 1000 to 1.

https://reviewboard.mozilla.org/r/208542/#review215044

Thanks for subbmit this patch! I believe the approach from comment 4 makes more sense to address this issue. Please give it a try.

Comment 6

[tera_1225]

Hi Ricky.
I see what you mean, that z-index property is a bit inelegant. I added it fixing 1360457 but it was only because I couldn't figure out a better way to get the headers to stay on top of the content lines when scrolling down. The screenshot illustrates this, with current mozilla-central, having just removed `z-index: 1000` from https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/assets/styles/RequestList.css#103 .
I'll have another look at solving this without the z-index. Let me know if you can think of a good way to keep it on top please.
Thanks for the review anyhow.
Regards, Mark.

Comment 7

[tera_1225]

Attachment: overlapping of headers and content with z-index property removed

Comment 8

[Ricky Chien [:rickychien] (inactive)]

Thanks for the quick feedback and uploading attachment :)

How about setting `z-index: 1` instead of removing it? I can see overlapping issue get fixed.

Comment 9

[tera_1225]

Comment on attachment 8937855 [details]
Bug 1425839 Reduced requests-list-headers-wrapper z-index from 1000 to 1.

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

Comment 10

[tera_1225]

No problem. Have pushed to review with that change, which indeed does solve both issues :) Good one!

Comment 11

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8937855 [details]
Bug 1425839 Reduced requests-list-headers-wrapper z-index from 1000 to 1.

https://reviewboard.mozilla.org/r/208542/#review215056

LGTM :) Thanks!

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/db61a884d137
Reduced requests-list-headers-wrapper z-index from 1000 to 1. r=rickychien

Comment 13

[Stefan Hindli [:stefan_hindli]]

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

Comment 14

[magicp]

This bug fix has verified in the latest Nightly build (20171222220251). Thanks!

Comment 15

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8937855 [details]
Bug 1425839 Reduced requests-list-headers-wrapper z-index from 1000 to 1.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1360457
[User impact if declined]: minor
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: see description
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: minor
[Why is the change risky/not risky?]: UI polish
[String changes made/needed]: none

Comment 16

[Gerry Chang [:gchang]]

Comment on attachment 8937855 [details]
Bug 1425839 Reduced requests-list-headers-wrapper z-index from 1000 to 1.

UI polish for netmonitor. Beta58+.

Comment 17

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2609db38f7d8

Comment 18

[Petruta Horea [:phorea], Desktop Test Engineering]

Verified as fixed using Firefox 58 beta 15 and latest Nightly 59.0a1 2018-01-08 under Win 10 64-bit, Ubuntu 16.04 64-bit and Mac OS X 10.13 with Dark, Light and Firebug themes.