Open Bug 1195461 Opened 9 years ago Updated 1 year ago

Deal with duplicate web component stylesheets in the style editor for chrome pages / browser toolbox

Categories

(DevTools :: Style Editor, enhancement, P1)

enhancement

Tracking

(firefox43 affected)

ASSIGNED
Tracking Status
firefox43 --- affected

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

Attachments

(1 obsolete file)

This seems to be caused by different XBL bindings including the same stylesheets. Updating either of the stylesheets *seems* to affect all the users, though I don't understand how that can be the case. Ideally, we should hide the duplicates for the user if and only if it's a chrome page they're looking at (websites could theoretically send different content for the same URL requested twice). We should then ensure that updates work seamlessly for all users of the stylesheet.
Inspector bug triage (filter on CLIMBING SHOES).
Priority: -- → P3
Product: Firefox → DevTools

I'd like to request we re-prioritize this, given that this is really bad now that we load a stylesheet for each instance of each component, rendering the stylesheet editor completely unusable on chrome pages.

Severity: normal → --
Priority: P3 → --
Summary: Deal with duplicate XBL stylesheets in the style editor for chrome pages / browser toolbox → Deal with duplicate web component stylesheets in the style editor for chrome pages / browser toolbox

This is a bit of WIP patch. It switches from a single styleSheet per
StyleSheetEditor to using a Set() of all the sheets with the same URI, and changes
StyleEditorUI.jsm to reuse editors based on the href of the sheet. Sheets
without a URI continue to have their own editors.

Questions I have on how to finish this:

  1. what to do if the page modifies one of the sheets via CSSOM. I think
    we would get events, though I could be wrong - but in any case, the question
    is more what to do when this happens. We could try to "detach" the stylesheet
    in question again, but I'm not sure how to do that or how to avoid renewing
    the confusion this patch aims to solve (ie: how would the user know which of
    the sheets is which, if they're being modified). I'm also not sure if this is
    common enough to warrant worrying about right now.
  2. what to do about source maps. I don't know how to test this well. I've
    tried to be reasonable in my changes, but chrome code doesn't use them so it's
    quite possible I've broken something without realizing. The easy way out might
    be not to unify the sheets if they're using source maps? But that'd probably
    reduce utility for web developers.
  3. highlighting nodes. I'm not sure how this works right now (I couldn't
    see any highlights when mousing over the editor), and if I can invoke the
    highlighter multiple times or if there's some other way to resolve this.
  4. automated tests. I assume I should add one; what would be a good template
    to base it on?

Finally, I'm really not sure who should review this - the last 3 months have
seen 3 patches to the style editor, all reviewed by pbro, who seems to have
moved on. :-(

On the plus side, this makes working with e.g. about:preferences and the browser
window reasonable again! Also, if you change a rule in one web component sheet
from the inspector or style editor, it is reflected across all instances. So
that's pretty neat.

Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Severity: -- → N/A
Type: defect → enhancement
Priority: -- → P1

The following patch is waiting for review from reviewers who are inactive or resigned from the review:

ID Title Author Reviewer Status
D75933 Bug 1195461 - use a single StyleSheetEditor instance for all stylesheets with the same URL to avoid duplicates for web component instances, r?gl!,jdescottes! Gijs jdescottes: Back Jun 25, 2023
rcaliman: Resigned from review

:Gijs, could you please find another reviewer or abandon the patch if it is no longer relevant?

For more information, please visit BugBot documentation.

Flags: needinfo?(gijskruitbosch+bugs)
Attachment #9150107 - Attachment is obsolete: true
Flags: needinfo?(gijskruitbosch+bugs)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: