#inspector-main-content defines `flex: 1 1 auto;` but isn't a flex item so this is inactive
Categories
(DevTools :: Inspector, task, P3)
Tracking
(firefox69 fixed)
| Tracking | Status | |
|---|---|---|
| firefox69 | --- | fixed |
People
(Reporter: pbro, Assigned: vanhagarsux)
Details
(Keywords: good-first-bug, Whiteboard: [lang=css])
Attachments
(2 files)
Screenshot 2019-06-11 at 15.24.29.png
- Open the inspector,
- open the Browser Toolbox,
- find the #inspector-main-content element in the inspector,
- select it
One of the css rules appied to this element defines flex: 1 1 auto; but since this element is not inside a flex container, this declaration is inactive and can be removed.
This is probably a left-over from old CSS code that used to be active but not longer is.
It's hard to check for useless css code in a large codebase such as devtools, but the new Inactive CSS tooltip seems to help a little with this, which is nice.
|
Reporter | |
Updated•4 years ago
|
Hi,
I'm an absolute beginner to Mozilla and Open Source and would like to work on this bug. I found the css rule in the "inspector.css" file of my local build. Is it just a matter of removing this rule, saving the changes to the modified "inspector.css" file and then uploading this patch as an attachment to Bugzilla for review?
Thank you!
|
|
Reporter | |
Comment 2•4 years ago
|
||
Thank you for looking into this!
Yes, basically what you said :)
just a matter of removing this rule
I think so, it looks like a left-over we forgot to remove, but to be totally sure, you should remove it, build your changes, and run firefox to test that everything works well (docs about getting, building and running the code here: https://docs.firefox-dev.tools/getting-started/build.html)
then uploading this patch as an attachment to Bugzilla for review
That is definitely a possibility and is exactly how we used to do things until recently. So you're welcome to do this.
However the Mozilla project now also uses Phabricator for code reviews. It's much easier to request a review, and much easier for reviewers to look at and merge the code. More info here: https://docs.firefox-dev.tools/contributing/code-reviews.html
I'm assigning the bug to you now.
Hi,
Thank you for all of the guidance and links for working on this bug. I've removed the left-over rule, re-built the changes and ran Firefox, making sure that everything still worked.
I've also created a Phabricator account and installed Arcanist. I'm now looking for someone to review my code. I found a list of people working on Inspector and you were included on that list. Would you be the best person to review my code?
Thank you,
J
|
|
Reporter | |
Comment 4•4 years ago
|
||
Thank you J for going this far! I suggest asking Gabriel for a review. His full name is Gabriel Luong, and his nickname on bugzilla/phabricator is gl.
|
||
Comment 5•4 years ago
|
||
It should flag me automatically if you add "r=gl" at the end of your commit message.
As for commit message, an example of how to format your commit message:
"Bug 1558656 - Remove inactive declaration in #inspector-main-content rule in inspector.css. r=gl"
One of the css rules appied to this element defines flex: 1 1 auto; but since this element
is not inside a flex container, this declaration is inactive and was removed.
Hi Gabriel,
Thank you for the info. I wasn't sure what to add to the "Test Plan" section of the arc diff editor file. Please let me know if anything looks off about my revision.
Thank you,
J
Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/78b7c6d54e0e Remove inactive declaration in #inspector-main-content rule in inspector.css. r=gl
|
||
Comment 9•4 years ago
|
||
| bugherder | ||
Description
•