Track Changes - refactor tracking of changes to rule selectors
Categories
(DevTools :: Inspector: Changes, enhancement, P2)
Tracking
(firefox67 fixed)
| Tracking | Status | |
|---|---|---|
| firefox67 | --- | fixed |
People
(Reporter: rcaliman, Assigned: rcaliman)
References
Details
Attachments
(3 files)
The patch for Bug 1525238, which was initially abandoned, became a blocker for Bug 1524547.
However, implementing the fix for Bug 1525238 broke the tracking of changes to rule selectors. Due to a limitation in the design of the Changes reducer, it became impossible to reuse the whole rule removal + whole rule addition approach to tracking selector changes (a gimmick, but it worked). Selector changes were not tracked atomically. The selector was a stateless and versionless property of the change object.
The rule id introduced by Bug 1525238 caused the old approach to fail by canceling itself out (rule remove + rule add). When non-changes like these happen, we clean them out intentionally so as not to pollute the Changes panel.
With Bug 1525238, the removed rule and the added rule have the same id (which is right, because the rule doesn't change, only its selector does). Previously, the selector text was used to build custom rule ids for each rule, thus ensuring their uniqueness.
The patches for this bug refactor the way selector changes are tracked to account for changes in this rule property over time.
|
Assignee | |
Comment 1•7 years ago
|
||
Refactors the Redux state for the Changes panel so that rules have
a selectors array instead of just a single selector string.
The selectors array represents the history of selector text the
rule has over time, not the actual list of multiple selectors it has
(multiple selectors are collapsed into a single comma-separated string
regardless of how many they are).
When the server logs changes, the rule's selector text is checked
against the history of previously logged selectors.
If the incoming selector is different than the first item in
the tracked rule's selectors array, it means the selector was renamed
so it is pushed onto the array (added to the history).
If it's the same, the whole array can be reduced to the original value
because it means that either:
- the selector didn't change between operations (ex: after declaration changes)
- the selector reverted back to its original name.
This selectors array is used in the React component in Part 2 of this
series to render any necessary diff view of the selector.
The old approach (whole rule removal + whole rule addition) is replaced
by this refactor. The introduction of the StyleRyleActor actor id from
the server as the rule id on the client in Bug 1525238 means that the
selector change can no longer behave like two distinct rules.
The actorID/rule id are preserved after selector renames. This necessary
for the some export options to work consistently (like Copy Rule with changes applied).
|
|
Assignee | |
Comment 2•7 years ago
|
||
Depends on D19828
Updates the React component of the Changes panel to show the diff of
a rule's selector if it has changed.
The previous implementation assumed that a selector rename meant a whole
rule removal (with the old selector) followed by a whole rule addition
(with the new selector).
This new implementation tracks the changes to the selector atomically.
The main benefit is that if the selector is renamed, the diff in the
Changes panel shows only this action and not the unchanged declarations.
The test was re-enabled and adjusted to account for this difference in behaviour.
This refactor was required in order to be compatible with Bug 1525238
which lays the ground work for matching rules from the client with rules
from the server. This is necessary in order to have consistent behaviour
for export options, like Copy Rule, which need to match the rule on the
server even if its selector was changed.
|
|
Assignee | |
Comment 3•7 years ago
|
||
Updates the Redux selector that generates styleesheets with changes to account for the new array structure for the history of the CSS Rule selector. Also fixes the xpcshell mock to account for the same thing.
With this change, selector renames also get captured in the output stylesheet (they weren't in the previous implementation)
|
|
Assignee | |
Comment 5•7 years ago
|
||
|
||
Comment 6•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/5ffc0cb683a1
https://hg.mozilla.org/mozilla-central/rev/abd87d1cd72d
https://hg.mozilla.org/mozilla-central/rev/63e277b9a83f
Description
•