Open Bug 1590392 Opened 5 years ago Updated 2 years ago

Cleanup css-logic modules found in devtools/shared and devtools/server/actors

Categories

(DevTools :: Inspector, task, P3)

task

Tracking

(Not tracked)

People

(Reporter: jdescottes, Unassigned)

References

Details

(See the comment at https://phabricator.services.mozilla.com/D49330?id=179492#inline-303983 for more context)

Today we have two modules called css-logic:

The devtools/shared/inspector css-logic module is mostly made of static CSS-related helpers. Those helpers are mainly used in the server (exception in text-property-editor.js). This module also exposes the styleeditor localization helper as l10n which a few modules use as a shortcut.

The tests for this module are found at devtools/shared/tests/mochitest and are chrome mochitests. https://searchfox.org/mozilla-central/source/devtools/shared/tests/mochitest

The devtools/server/actors css-logic module exposes mostly 2 things:

  • a CssLogic class that has to be instantiated, and that keeps a sort of state depending on which "element" it currently highlights. As the JsDoc mentions, part of the class depends on the currently highlighted element, part doesn't...
  • some static helpers exposed on CssLogic's constructor (eg getBindingElementAndPseudo). They are typically imported from devtools/shared/inspector/css-logic.js and just exposed via an alias here

The goals of this bug are:

  • move all the static helpers to a consistent location and stop aliasing them on the actor's file
  • potentially split the helpers in several modules if it makes sense. It seems like we could have one module about selectors & paths, one module about CSS prettifying...
  • make sure the tests are right next to the implementation
  • find a more suitable name for the CssLogic class. It really isn't clear what this class is supposed to do. If some methods in the class are actually not relying on the state of the instance and can be moved to static helpers, move them out.
  • potentially split out the subclasses defined in actors' css-logic.js in dedicated modules
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.