1597248 - Some selected nodes are hard to read in the markup view

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P3)

Description

[Julian Descottes [:jdescottes]]

STRs:

• open a page with hidden iframe (eg https://jdescottes-hiddeniframe.glitch.me/)
• open inspector
• expand the iframe
• select the "#document" element

Expected result: The text #document should be readable
Actual result: The text color is not updated and is hard to read against the blue background

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: image.png

Comment 2

[Julian Descottes [:jdescottes]]

I guess this affects only #document, #document-fragment and #shadow-root tags

Comment 3

[Julian Descottes [:jdescottes]]

This seems to be a simple CSS issue:
https://searchfox.org/mozilla-central/rev/524bed6dfbc5ae21c62632d83b7573448b29e0ac/devtools/client/themes/markup.css#406-419

.not-displayed .tag {
  color: var(--markup-hidden-tag-color);
}

/* Selected nodes in the tree should have light selected text.
   theme-selected doesn't work in this case since the text is a
   sibling of the class, not a child. */
.theme-selected ~ .editor,
.theme-selected ~ .editor.comment,
.theme-selected ~ .editor .theme-fg-color1,
.theme-selected ~ .editor .theme-fg-color2,
.theme-selected ~ .editor .theme-fg-color3 {
  color: var(--theme-selection-color);
}

Most elements of the markup view have either .comment or .theme-fg-color1, .theme-fg-color2, .theme-fg-color3 applied, which has higher precedence than their .not-displayed rule. However this is not the case for the #document nodes. Their markup is just

<span xmlns="http://www.w3.org/1999/xhtml" class="tag" tabindex="-1">#document</span>

So the selectors that apply are .not-displayed .tag and .theme-selected ~ .editor. Sibling selectors count as single class selectors, so .not-displayed .tag has high precedence. We should either add another classname or fix the CSS.

Comment 4

[Florens Verschelde :fvsch]

It's not a specificity or precedence issue exactly, it's that we have this DOM structure:

<span class="editor">
  <span class="tag" tabindex="0">#document</span>
</span>

To also match the .tag child, we could add this selector to the list:

 .theme-selected ~ .editor,
 .theme-selected ~ .editor.comment,
+.theme-selected ~ .editor .tag,
 .theme-selected ~ .editor .theme-fg-color1,
 .theme-selected ~ .editor .theme-fg-color2,
 .theme-selected ~ .editor .theme-fg-color3 {
   color: var(--theme-selection-color);
 }

Comment 5

[Florens Verschelde :fvsch]

A small example to explain the order of inheritance versus specificity:

<style>
/* 0,0,1 specificity */
span {
  color: green;
}

/* 1,3,0 specificity */
#container.big.bad.wolf {
  color: red !important;
}
</style>

<div id="container" class="big bad wolf">
  <span>I'm going to be green, not red</span>
</div>

Comment 6

[Julian Descottes [:jdescottes]]

Ah good point, I missed that!

Comment 7

[Julian Descottes [:jdescottes]]

Adding good first bug, the fix is suggested in comment 4, the file to update is https://searchfox.org/mozilla-central/source/devtools/client/themes/markup.css#414

Comment 8

[josh]

Hi! I'm happy to take this on. As a first time contributor I'll need a little direction on how to get setup and the process for submitting the fix. Is there a guide somewhere I can look at?

Comment 9

[Julian Descottes [:jdescottes]]

Hi Josh! Thanks for offering to work on this, assigning the bug to you :)

Is there a guide somewhere I can look at?

You can take a look at https://docs.firefox-dev.tools/ for the DevTools developer guide.
It will explain how to get the code, build Firefox and test your changes.

However, when you are ready to submit a patch I advise to switch to https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html which has much more accurate information on how to setup the code review tools, how to submit your patch etc...

And if you have any question, you can find us on Slack at https://devtools-html-slack.herokuapp.com/ or you can use the "Request information from" checkbox, just below the comment box.

Comment 10

[josh]

Wonderful, thanks for the direction! I'll be able to take a look this weekend.

Comment 11

[Patrick Brosset <:pbro>]

Hi Josh, just checking in on this bug, since there hasn't been activity over the past 2 months.
Are you still interested in working on it?
Please let us know so we can make it available to others if needed.

Comment 12

[josh]

(In reply to Patrick Brosset <:pbro> from comment #11)

Hi Josh, just checking in on this bug, since there hasn't been activity over the past 2 months.
Are you still interested in working on it?
Please let us know so we can make it available to others if needed.

So sorry! I completely forgot about this. I don't think I'll be able to get to it in the next few weeks either. Please assign it to someone else. Apologies!

Comment 13

[Maria Terrenal (:mdterrenal)]

Hi! I would love to give this issue a go and make my first contribution to Mozilla if this issue hasn't been resolved yet.

Comment 14

[Patrick Brosset <:pbro>]

Thanks Maria for your interest. The bug is yours!
Please take a look at comment 9 for how to get started. And then comment 3, comment 4 and comment 5 to know what to change in order to fix this bug.
Please don't hesitate to post comments here to ask any questions you may have.

Comment 15

[Maria Terrenal (:mdterrenal)]

Thank you for the guidance! I'll be sure to update you on my progress and ask more questions if I have any.

Comment 16

[Patrick Brosset <:pbro>]

Hi Maria, any updates on this? Do you have any problems or questions we could help you with?

Comment 17

[Maria Terrenal (:mdterrenal)]

Hi Patrick, my apologies for the delay! I had some trouble setting up my development environment but I'll be working on this bug this week. I'll update you on my progress by the end of the week and I'll be sure to let you know if I come across any problems.

Comment 18

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Hi Maria, are you still interested in fixing this bug? Please don't hesitate to ask questions if you need any help.

Honza

Comment 19

[Julian Descottes [:jdescottes]]

We can probably unassign this one.

Comment 20

[manekenpix]

I'd like to work on this one.

Comment 21

[Julian Descottes [:jdescottes]]

(In reply to manekenpix from comment #20)

I'd like to work on this one.

Thanks, assigning this bug to you.

If you need help to get started, you can either contact us by posting a comment here (use the "Request information from" checkbox below to make sure we are notified), or join our public chat room at https://chat.mozilla.org/#/room/#devtools:mozilla.org

Comment 22

[manekenpix]

Attachment: Bug 1597248 - Fixed selected nodes are hard to read in markup view

Comment 23

[manekenpix]

I added the suggested fix in comment 4.
Please let me know if there's anything else I can do.

Comment 24

[manekenpix]

(In reply to manekenpix from comment #23)

I added the suggested fix in comment 4.
Please let me know if there's anything else I can do.

Comment 25

[manekenpix]

Sorry about that last comment.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/244e500071c2
Fixed selected nodes are hard to read in markup view r=jdescottes

Comment 27

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/244e500071c2

Comment 28

[Oana Botisan, Desktop Release QA]

I managed to reproduce the issue using an older version of Nightly on macOS 10.13. I retested everything on the same OS using Firefox 84.0 and Nightly 85.0a1. The issue is not reproducing anymore.