Closed Bug 1788030 Opened 2 years ago Closed 2 years ago

Only "first" media query from nested media queries is displayed in the Media Queries sidebar

Categories

(DevTools :: Style Editor, defect, P2)

defect

Tracking

(firefox106 fixed)

RESOLVED FIXED
106 Branch
Tracking Status
firefox106 --- fixed

People

(Reporter: nchevobbe, Assigned: nchevobbe)

References

Details

Attachments

(1 file)

Steps to reproduce

  1. Navigate to data:text/html,<meta charset=utf8><style>@media screen {@media (min-width: 1px) {@media (min-height: 1px) {@media (max-width: 9999px) {@media (max-height: 9999px) {body {background: tomato;}}}}}}</style>Tomato
  2. Open the style editor and make sure the media queries sidebar is displayed

Expected results

The sidebar has the whole list of nested media queries

Actual results

Only the first media query is displayed (screen)


Here's the content of the stylesheet nicely formatted:

@media screen {
  @media (min-width: 1px) {
    @media (min-height: 1px) {
      @media (max-width: 9999px) {
        @media (max-height: 9999px) {
          body {
            background: red;
          }
        }
      }
    }
  }
}
No longer blocks: 1731612
No longer depends on: 1751417
Blocks: 1788033

Setting as P2 as we might pick it up soon.

Severity: -- → S3
Priority: -- → P2

In StyleSheetsManager _getMediaRules, we were only looping
over document.cssRules, and as a result were missing potential
nested media queries.
For each rule, we now go other its potential cssRules, recursively,
to gather all media query rules.
Test cases are added for the client to make sure that those nested
rules do show up in the StyleEditor media queries sidebar.

Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/342c3364a88c
[devtools] Display nested media queries in media sidebar. r=jdescottes.
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 106 Branch

== Change summary for alert #35326 (as of Wed, 07 Sep 2022 21:44:52 GMT) ==

Regressions:

Ratio Test Platform Options Absolute values (old vs new)
14% damp custom.styleeditor.open.DAMP windows10-64-shippable-qr e10s fission stylo webrender 771.45 -> 879.01
13% damp custom.styleeditor.open.DAMP linux1804-64-shippable-qr e10s fission stylo webrender 801.07 -> 904.38
12% damp custom.styleeditor.open.DAMP linux1804-64-shippable-qr e10s fission stylo webrender-sw 789.05 -> 887.52
12% damp custom.styleeditor.open.DAMP macosx1015-64-shippable-qr e10s fission stylo webrender-sw 723.97 -> 812.24
12% damp custom.styleeditor.open.DAMP windows10-64-shippable-qr e10s fission stylo webrender-sw 785.04 -> 880.69

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=35326

Nicolas: This seems to have a visible impact on custom styleeditor open. This is the test we added recently and which has a lot of stylesheets to display.

Do you think that similarly to what we did for highlighting, we should also make the retrieval of media queries lazy (only fetch them when the stylesheet gets selected?)

If yes, we could file a follow up bug?

Flags: needinfo?(nchevobbe)

(In reply to Julian Descottes [:jdescottes] from comment #5)

== Change summary for alert #35326 (as of Wed, 07 Sep 2022 21:44:52 GMT) ==

Regressions:

Ratio Test Platform Options Absolute values (old vs new)
14% damp custom.styleeditor.open.DAMP windows10-64-shippable-qr e10s fission stylo webrender 771.45 -> 879.01
13% damp custom.styleeditor.open.DAMP linux1804-64-shippable-qr e10s fission stylo webrender 801.07 -> 904.38
12% damp custom.styleeditor.open.DAMP linux1804-64-shippable-qr e10s fission stylo webrender-sw 789.05 -> 887.52
12% damp custom.styleeditor.open.DAMP macosx1015-64-shippable-qr e10s fission stylo webrender-sw 723.97 -> 812.24
12% damp custom.styleeditor.open.DAMP windows10-64-shippable-qr e10s fission stylo webrender-sw 785.04 -> 880.69

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=35326

Nicolas: This seems to have a visible impact on custom styleeditor open. This is the test we added recently and which has a lot of stylesheets to display.

Do you think that similarly to what we did for highlighting, we should also make the retrieval of media queries lazy (only fetch them when the stylesheet gets selected?)

If yes, we could file a follow up bug?

Yes, that's something we could do for sure. I'm a bit surprised to see such impact on the results though, I'll check if I'm doing unnecessary work when traversing the rules

Flags: needinfo?(nchevobbe)
Regressions: 1790595
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: