#inspector-main-content defines `flex: 1 1 auto;` but isn't a flex item so this is inactive

RESOLVED FIXED in Firefox 69

Status

task
P3
normal
RESOLVED FIXED
2 months ago
2 months ago

People

(Reporter: pbro, Assigned: vanhagarsux)

Tracking

({good-first-bug})

unspecified
Firefox 69
Points:
---

Firefox Tracking Flags

(firefox69 fixed)

Details

(Whiteboard: [lang=css])

Attachments

(2 attachments)

  • 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.

Keywords: good-first-bug
Priority: -- → P3
Whiteboard: [lang=css]

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!

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.

Assignee: nobody → vanhagarsux
Status: NEW → ASSIGNED

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

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.

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
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69
You need to log in before you can comment on or make changes to this bug.