1753196 - Inspector - inherited styles crossed out incorrectly when important is used

Status: VERIFIED FIXED

(DevTools :: Inspector: Rules, defect, P2)

Description

[Piotr Olczak]

Attachment: firefox_inspector_bug.png

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:96.0) Gecko/20100101 Firefox/96.0

Steps to reproduce:

Note:
My issue sounds very similar to https://bugzilla.mozilla.org/show_bug.cgi?id=1087273 but I was unable to reproduce it using the previous reporter's example.

Steps to reproduce:

1. Go to https://jsfiddle.net/polczak_ittouch/a64ovpdn/8/ for a prepared playground or load a webpage with the below HTML and CSS.

HTML

<div>
  <ul>
    <li>Firefox bug</li>
    <li>test</li>
  </ul>
</div>

CSS

div {
  font-size: 50px !important;
}

ul {
  font-size: 14px;
}

2. Open the Firefox Inspector and inspect any of the two li elements
3. Look at the Rules tab (or the middle section if using 3-pane inspector)

Actual results:

See the attached image for reference.

The smaller font-size is correctly applied to the li elements (it takes precedence because !important doesn't affect inherited CSS - the rule from the closest ancestor is in effect) which is reflected correctly in the Computed tab of the Inspector. However in the Rules tab the smaller font-size rule is crossed out suggesting that it's not taking effect. If you toggle the checkbox next to it on and off you can see that it is indeed taking effect, but the rule is crossed out all the time, unless you click the checkbox next to the other rule with !important.

So in short: the Rules tab "thinks" that !important is changing the precedence of inherited CSS even though it does not.

Expected results:

The CSS rules should be crossed out correctly.

Comment 1

[Piotr Olczak]

Attachment: chrome_inspector_correct.png

For reference: for the same HTML and CSS Google Chrome Inspector does not have this issue

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:jdescottes, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 3

[Julian Descottes [:jdescottes]]

Thanks for filing, sounds like an old bug which dates back to an initial fix around similar issues with important & inherited properties: https://bugzilla.mozilla.org/show_bug.cgi?id=979574

The relevant code is at https://searchfox.org/mozilla-central/rev/3de1b6791ea569cdf7773f7cd512cf2e6cc1d3f0/devtools/client/inspector/rules/models/element-style.js#343-357:

      let overridden;
      if (
        earlier &&
        computedProp.priority === "important" &&
        earlier.priority !== "important" &&
        (earlier.textProp.rule.inherited ||
          !computedProp.textProp.rule.inherited)
      ) {
        // New property is higher priority. Mark the earlier property
        // overridden (which will reverse its dirty state).
        earlier._overriddenDirty = !earlier._overriddenDirty;
        earlier.overridden = true;
        overridden = false;
      } else {
        overridden = !!earlier;
      }

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1753196 - [devtools] RuleView: inherited properties marked as overridden incorrectly

When both the current and the earlier props for a given property name are inherited, the current prop should not be marked as overridden if the earlier prop was marked as !important.
!important should only come into play if the property where it was set is not inherited.
Regardless of this, properties are considered in descending order of specificity, so the earlier rule should take precedence.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bc480c50be30
[devtools] RuleView: inherited properties marked as overridden incorrectly r=nchevobbe

Comment 6

[Natalia Csoregi [:nataliaCs]]

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

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jdescottes, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 8

[Julian Descottes [:jdescottes]]

Very old issue, can ride the trains.

Comment 9

[Monica Chiorean]

I do not manage to reproduce the issue on Win10 using a build without fix 98.0a1 (20220125190421).
Could it be possible for you to check if the issue is still reproducing on your side on latest Beta 99.0 (https://archive.mozilla.org/pub/firefox/candidates/99.0-candidates/ )? Thank you.

Comment 10

[Piotr Olczak]

I can reproduce the issue on Linux Mint on version 98.0.2, but not the latest Beta you linked. Are you sure that this Beta build is without the fix? It was supposed to be slated for Firefox 99 release, so I don't see why it shouldn't be in the Beta version.

Comment 11

[Julian Descottes [:jdescottes]]

Short version of the STRs

• open https://jsfiddle.net/polczak_ittouch/a64ovpdn/8/
• in the white rectangle on the bottom right, right click on "Firefox bug", select "Inspect"
• in the DevTools Inspector panel, font-size: 50px !important; should be greyed out and have a strike

I just tested on 98, the issue is there, while it's fixed on 99 or 100.
As Piotr said, it should be fixed on 99.

Comment 12

[Piotr Olczak]

Oh sorry, I misunderstood what Monica meant by "not managing to reproduce the issue". Yes, it's all good now on 99.

Comment 13

[Monica Chiorean]

Marking issue as verified based on previous comments. Thank you both.