Closed Bug 1425839 Opened 6 years ago Closed 6 years ago

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

Categories

(DevTools :: Netmonitor, defect, P3)

defect

Tracking

(firefox-esr52 unaffected, firefox57 unaffected, firefox58 verified, firefox59 verified)

VERIFIED FIXED
Firefox 59
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- verified
firefox59 --- verified

People

(Reporter: magicp.jp, Assigned: tera_1225)

References

Details

Attachments

(3 files, 1 obsolete file)

Attached image 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
Blocks: 1360457
Has Regression Range: --- → yes
Has STR: --- → yes
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
Attachment #8937702 - Attachment is obsolete: true
Attachment #8937702 - Flags: review?(rchien)
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.
Assignee: nobody → tera_1225
Status: NEW → ASSIGNED
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.
Attachment #8937855 - Flags: review?(rchien) → review-
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.
Flags: needinfo?(rchien)
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.
Flags: needinfo?(rchien)
No problem. Have pushed to review with that change, which indeed does solve both issues :) Good one!
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!
Attachment #8937855 - Flags: review?(rchien) → review+
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/db61a884d137
Reduced requests-list-headers-wrapper z-index from 1000 to 1. r=rickychien
https://hg.mozilla.org/mozilla-central/rev/db61a884d137
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
This bug fix has verified in the latest Nightly build (20171222220251). Thanks!
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
Attachment #8937855 - Flags: approval-mozilla-beta?
Comment on attachment 8937855 [details]
Bug 1425839 Reduced requests-list-headers-wrapper z-index from 1000 to 1.

UI polish for netmonitor. Beta58+.
Attachment #8937855 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.