Adding a new CSS rule shows the temporary generated selector as removed in the Changes panel

VERIFIED FIXED

Status

defect
P1
normal
VERIFIED FIXED
2 months ago
27 days ago

People

(Reporter: vlucaci, Assigned: rcaliman)

Tracking

(Blocks 1 bug)

Trunk
Desktop
All
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed, firefox68 verified)

Details

Attachments

(2 attachments)

Reporter

Description

2 months ago

Affected versions

  • 67.0b8
  • 68.0a1(2019-04-04)

Affected platforms

  • Windows 10x64
  • macOS 10.14
  • Ubuntu 16.04

Steps to reproduce

  1. Launch FF.
  2. Go to any website (e.g Wikipedia.com)
  3. Activate the Inspector and the Changes Pane.
  4. Right click in the second pane to activate the context menu and choose Add New Rule while having selected anything from the left pane.
    5.Change the default highlighted value to anything else and commit the change.

Expected result

  • In the changes pane, only the new rule is displayed with +.

Actual result

  • In the changes pane, the rule added by the user is with + , and the value that was highlighted in the text field is displayed with - .

Additional notes

  • The issue here stems in the fact that the text that was contained in the text field after clicking the Add New Rule button from context menu behaves as a set declaration that has been modified when in fact a completely new rule has been added so the -(red) information is redundant.
Assignee

Comment 1

2 months ago

Thanks for filing! This is indeed a bug, but it's a tricky architectural issue for now.

The root cause is that using the "Add New Rule" action (button or context menu) will immediately invoke the injection of a new CSS rule with the auto-generated selector matching the element. When changing the selector and committing the value, the previously injected rule is updated, hence why the removed old selector shows up as removed when the expectation is for just the new selector to show as added.

Basically, adding a new rule is a two-step process which get aggregated in the Changes panel as an added, then edited CSS rule.

The solution in the current Rule view is to create a mock Rule model, push it to the ElementStyle list of matched rules (though it doesn't exist yet), use it to update the Rule view UI, then send it to the PageStyle actor only once the selector input field is committed.

The existing architecture for the Rules view makes this tricky. The mock Rule model needs a real RuleEditor instance, forced updates and recalculations of the ElementStyle rules, and the concert of events fired which re-render the UI needs alterations.

This will be substantially easier with the new Rules view currently being worked on in bug 1518615. I'll mark this bug as a P3 to put it as a reminder on the backlog for now. We'll attempt to fix this in the new Rules view. Fixing this in the old (current) Rules view involves added complexity that's destined to be short-lived.

Blocks: track-changes
No longer blocks: 1515875, 1503920
Priority: -- → P3
See Also: → dt-rules
Assignee

Updated

2 months ago
Summary: Unset default value when adding a new rule is displayed in Changes Pane as if it was modified → Adding a new CSS rule shows the temporary generated selector as removed in the Changes panel
Assignee

Comment 2

2 months ago

Adding a new CSS rule renders in the Changes panel with the temporary selector as being removed even though the user never committed the temporary selector.

Only the manually typed selector should show in the Changes panel.

Assignee

Updated

a month ago
Assignee: nobody → rcaliman
Status: NEW → ASSIGNED
Priority: P3 → P1
Assignee

Comment 3

29 days ago

The patch for Bug 1542213 which landed in Nightly should fix this bug also.
@Vlad, can you please confirm if the issue is indeed resolved and close the bug if so?

Flags: needinfo?(vlad.lucaci)
Reporter

Comment 4

29 days ago

Hello.

Sure thing @Razvan.

Confirming that this issue is fixed in 68.0a1 (2019-04-23)(20190423221610) as part of Bug 1542213.

Flags: needinfo?(vlad.lucaci)
Reporter

Updated

29 days ago
Status: ASSIGNED → RESOLVED
Last Resolved: 29 days ago
Resolution: --- → FIXED
Reporter

Updated

29 days ago
Status: RESOLVED → VERIFIED
Assignee

Comment 5

29 days ago

Thank you, Vlad! I will proceed with requesting an uplift of the patch for Bug 1542213 to Beta.

You need to log in before you can comment on or make changes to this bug.