1993440 - UserProperties Map keys (<Rule actorID>:<declaration name>) can lead to erroneous data

Status: NEW

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

Description

[Nicolas Chevobbe [:nchevobbe]]

Steps to reproduce

1. Navigate to data:text/html,<meta charset=utf8><style>h1 { color: blue; color: red; }</style><h1>hello
2. Inspect the <h1> element
3. In the Rules view change red to gold

Expected results

The last declaration (now color: gold;) gets a green border, indicating that it was modified

Actual results

The last declaration (color: gold;) does gets a green border, but so do color: red, although it wasn't modified

---

The key is computed in https://searchfox.org/firefox-main/rev/2d38cd5bd7d7958137dd5b4f97aef2f7185a0eb1/devtools/client/inspector/rules/models/user-properties.js#76-78

getKey(style, name) {
  return style.actorID + ":" + name;
}

And called from different callsites , e.g. https://searchfox.org/firefox-main/rev/2d38cd5bd7d7958137dd5b4f97aef2f7185a0eb1/devtools/client/inspector/rules/models/user-properties.js#39-45,48-50

/**
 * Set a named property for a given CSSStyleDeclaration.
 *
 * @param {CSSStyleDeclaration} style
 *        The CSSStyleDeclaration against which the property is to be mapped.
 * @param {String} name
 *        The name of the property to set.
...
 */
setProperty(style, name, userValue) {
  const key = this.getKey(style, name);

The issue is that those functions are not called with a CSSStyleDeclaration (note that it's probably shouldn't be that to start with, we don't have direct access to this object from the client), but with a Rule front, e.g. https://searchfox.org/firefox-main/rev/2d38cd5bd7d7958137dd5b4f97aef2f7185a0eb1/devtools/client/inspector/rules/models/text-property.js#196

this.userProperties.setProperty(this.rule.domRule, this.name, value);

this means that if you have 2 declarations in a Rule, they will have the same key, which might cause further issues.

---

We should try to find something we could use that would better represent a given declaration.
Note that we can't really use the TextProperty id as it's not stable (e.g. if you select another node and then select the previous node back, your text properties were re-created with new ids)

Comment 1

[Julian Descottes [:jdescottes]]

Maybe instead of relying on a model on the frontend we could rely on the architecture of the changes panel and retrieve the changes from the server.