Closed Bug 1699648 Opened 3 years ago Closed 3 years ago

Empty style tab in Inspector

Categories

(DevTools :: Inspector, defect, P2)

Product:
DevTools
Component:
Inspector
Version:
Firefox 86
Type:
defect

Tracking

(firefox-esr78 wontfix, firefox86 wontfix, firefox87 wontfix, firefox88 fixed, firefox89 fixed)

Status:
RESOLVED FIXED
Milestone:
89 Branch
Tracking Flags:
Tracking Status
firefox-esr78 --- wontfix
firefox86 --- wontfix
firefox87 --- wontfix
firefox88 --- fixed
firefox89 --- fixed

People

(Reporter: bugzilla, Assigned: jdescottes)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(3 files)

Screenshot of the open dev tools
50.48 KB, image/png
Details
console output
6.72 KB, text/x-log
Details
Bug 1699648 - [devtools] Use the proper principal to fetch the text of imported stylesheets
48 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-beta+
Details | Review

I’m working on a rather complex set-up with custom SSL certificate, variable fonts, CSS custom properties, you name it.

There is a set of elements, that will never show any output in the style pane of the Inspector. The layout and calculated style tabs are fine. Just the one pane is empty.

Unfortunately I was not able to reduce the problem to a simple test case. I’ve tried removing all styles apart from an empty block selecting the the element, and that’s enough in my set-up to trigger the error.

I’ve attached a screenshot of the open dev tools and the output that the browser console showed.

Attached file console output —

Thanks for the report. I assume it's not possible to give us access to this website?
Could you open the Browser Console (Tools > Web Developer > Browser Console) and attach here the errors you see?

The best way to reduce the amount of logs would be to open the Browser Console before triggering the issue, clearing the logs and then follow the steps you usually use to trigger the blank panel.

Thanks!

Flags: needinfo?(bugzilla)

Sorry I didn't see you already provided the console log! Thanks a lot

Flags: needinfo?(bugzilla)

The site is a rewrite of https://codepoints.net, but I’ll need some time to get it live, i.e., get the current state properly running on another machine than just my PC. As soon as it will be live (and assuming the bug is present on the live site), I’ll post a link here, but that might as well take two more weeks.

So we crash at https://searchfox.org/mozilla-central/rev/31b583bbcc9cfaacd93e785c05dda0e87d7cca4e/devtools/server/actors/utils/style-utils.js#171-183

function getTextAtLineColumn(text, line, column) {
  let offset;
  if (line > 1) {
    const rx = new RegExp(
      "(?:[^\\r\\n\\f]*(?:\\r\\n|\\n|\\r|\\f)){" + (line - 1) + "}"
    );
    offset = rx.exec(text)[0].length;
  } else {
    offset = 0;
  }
  offset += column - 1;
  return { offset: offset, text: text.substr(offset) };
}

Precisely on offset = rx.exec(text)[0].length;. It would be interesting to see the values of text and line when rx.exec(text) is null.

(In reply to Manuel Strehl from comment #4)

The site is a rewrite of https://codepoints.net, but I’ll need some time to get it live, i.e., get the current state properly running on another machine than just my PC. As soon as it will be live (and assuming the bug is present on the live site), I’ll post a link here, but that might as well take two more weeks.

Thanks!

Maybe we can fix it before that. We could just make the method mentioned above safer, but I really wonder why it fails.
I guess either we get an incorrect cssText for the stylesheet, or we retrieved a wrong lineNumber which is incompatible with the provided text.

Are you using CSS sourcemaps on this project?

Unfortunately not. But I’m using plain old CSS. What’s apparently necessary is, that the selector is in an @import-embedded stylesheet. I reduced it literally to:

main.css:
@import "_crosslinks.css"

_crosslinks.css:
.tiles .cp .title {}

and that’s enough to crash. (I’ve also deactivated all Firefox addons and tested w/o them.)

I’ve reduced it to the following test case:

  • CSS as above.
  • HTML:
<html>
  <head>
    <link rel="stylesheet" href="/static/css/main.css">
    </head>
  <body>
    <ol class="tiles">
      <li><a class="cp" href="/U+0B85"><span class="title">Tamil Letter A</span></a></li>
    </ol>
  </body>
</html>
  • served via HTTPS, but with a self-signed certificate

Is it possible, that the class name "title" is interpreted somewhere wrongly, maybe as a property name or so? In the same _crosslinks.css are usually other CSS declarations as well, and all others work like a charm.

(Nope, that’s a dead end. Changed all involved class names by adding an “x”, and the error persists.)

Thanks!

Tried to create a testcase with the information you provided: https://bug-1699648.glitch.me/
But I can't get the issue yet. Can you give it a try to see if you reproduce with this test case? Otherwise it might be indeed about the way the files are served?

I don’t see the same issue, but something else, that I noticed, too: The empty selector is not always shown, and when it is, it’s always with a short delay. The browser console is empty, though (apart from a favicon error).

Screenshots:
https://dumps.codepoints.net/Screenshot_20210319_145748.png
https://dumps.codepoints.net/Screenshot_20210319_145817.png

Apparently, a browser restart fixed the problem. I can rule out a pending Firefox update. It was a “business as usual” day. What I did yesterday, was opening and closing the DevTools repeatedly, so that alone was not enough to fix it.

I’ve put the beta version of the site online: https://beta.codepoints.net/basic_latin

But I cannot reproduce the bug there or under the original domain (my local machine, host codepoints.next, self-signed certificate).

Sorry for spamming this bug! The next browser restart just moved the problem to two other sets of elements, that will now exhibit the same behaviour.

AFAICT, the problem is always in the part of the styles, that I edited last. The styles are applied just fine when I reload the page, but the DevTools crash on the newly added declarations in @import-fetched stylesheets. That might also explain, why I cannot recreate the issue on beta.codepoints.net, because there I do no editing.

No need to apologize, this is not spam!

You are absolutely correct, this is only happening when editing an imported stylesheet.
STRs:

  • open a website which you can author and uses an imported stylesheet
  • inspect an element with styles from the imported stylesheet (note the line number of the inspected rule)
  • update the stylesheet so that the "line number" of the rule is now higher than the previous line count of the stylesheet
  • reload the page and try to inspect the element

ER: The RuleView should still display the correct content
AR: Blank RuleView

If you want a small test case, you can remix the following glitch test case: https://glitch.com/edit/#!/bug-1699713
Then play around with the content of _crosslinks.css.

It looks like there is some aggressive caching happening with imported stylesheets and devtools keep getting a cached version of the stylesheet.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Blocks: 1700010

Regressed by Bug 1639154 on mozilla-central, which landed in FF78.

However the issue only occurs when privacy.partition.network_state is set to true. And this preference was only flipped to true on the Release channel in Bug 1681330 (FF85)

Regressed by: 1639154
Has Regression Range: --- → yes

We lack an ownerNode for imported stylesheets and therefore we fail to use the proper principal at https://searchfox.org/mozilla-central/rev/1758450798ae14492ba28b695f48143840ad6c5b/devtools/server/actors/style-sheet.js#138

I think we can simply loop on sheet.parentStyleSheet until we get to the top stylesheet that has an ownerNode.

Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Flags: needinfo?(emilio)
Flags: needinfo?(amarchesini)

Sorry didn't mean to ni? here

Flags: needinfo?(emilio)
Flags: needinfo?(amarchesini)

Imported stylesheets don't have an ownerNode property, it needs to be retrieved on the parentStylesheet (potentially recursively).
Add a helper to resolve the ownerNode and use it retrieve the correct principal to fetch the stylesheet.
Add a test case simulating a stylesheet update.

Blocks: 1699713
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8c9009b2cb96
[devtools] Use the proper principal to fetch the text of imported stylesheets r=nchevobbe

https://hg.mozilla.org/mozilla-central/rev/8c9009b2cb96

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
status-firefox89: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 89 Branch
Severity: -- → S2
status-firefox86: --- → wontfix
status-firefox87: --- → wontfix
status-firefox88: --- → affected
OS: Linux → All
Priority: -- → P2
Hardware: x86_64 → All
status-firefox-esr78: --- → wontfix
Flags: in-testsuite+

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.

Flags: needinfo?(jdescottes)

Comment on attachment 9211283 [details]
Bug 1699648 - [devtools] Use the proper principal to fetch the text of imported stylesheets

Beta/Release Uplift Approval Request

  • User impact if declined: The inspector UI starts breaking (incorrect information displayed, blank panel etc...) for developers using stylesheets loaded via @import (which is used by major CMS such as Drupal). We have seen several duplicate Bugs for this issue, as well as users reporting it on Discourse https://discourse.mozilla.org/t/rules-empty-in-developers-tools/51745/12
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Trivial javascript change, devtools only. We need to retrieve the "ownerNode" of a stylesheet object. For imported stylesheets this can be null, and instead needs to be retrieved on the "parentStylesheet" (potentially recursively if there are nested @import steps).

This logic was already used to handle CSS sourcemaps, we now use it also in another place to retrieve the correct CSS text for a stylesheet

  • String changes made/needed:
Flags: needinfo?(jdescottes)
Attachment #9211283 - Flags: approval-mozilla-beta?

Comment on attachment 9211283 [details]
Bug 1699648 - [devtools] Use the proper principal to fetch the text of imported stylesheets

Longstanding issue with a number of reported dupes. Thanks for including tests. Approved for 88.0b5.

Attachment #9211283 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: