Closed Bug 1736412 Opened 3 years ago Closed 3 years ago

"Add new rule" button stops working when style node is deleted or moved

Categories

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

Product:
DevTools
Component:
Inspector: Rules
Version:
Firefox 92
Type:
defect

Tracking

(firefox97 verified)

Status:
VERIFIED FIXED
Milestone:
97 Branch
Tracking Flags:
Tracking Status
firefox97 --- verified

People

(Reporter: dev, Assigned: colin.cazabet, Mentored)

Details

Attachments

(2 files)

Screen recording of the reproduction steps
5.53 MB, video/quicktime
Details
Bug 1736412 - [devtools] Fix 'Add new rule' button when style node is deleted or moved. r=jdescottes
48 bytes, text/x-phabricator-request
Details | Review

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/94.0.4606.81 Safari/537.36

Steps to reproduce:

  1. Open any page in Firefox. For example, the following file content can be used for reproducing:
    <html>
    <body>
    <div>This is some placeholder content.</div>
    </body>
    </html>

  2. Open the dev tools and select an element in the inspector.

  3. In the rules view, press the "Add new rule" button. A new (empty) style declaration is added in the rules view. Also, an empty <style> element is added to the end of the DOM.

  4. Delete this new style element (or move it to a different place in the DOM using drag-and-drop).

  5. Press the "Add new rule" button again.

Actual results:

When pressing the "Add new rule" button at step 5, nothing happens (even when pressing it repeatedly).

Expected results:

The same as what happens when the button is pressed in step 3:

A new (empty) style declaration should be added, and a new empty <style> element should be added to the end of the DOM.

Thanks for the report, I can confirm the bug happens with the current Nightly, and is probably not a recent regression.
Setting P3/S3 because STRs are a bit complex, but it would really be nice to fix, especially considering there is 0 user feedback.

Severity: -- → S3
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Flags: needinfo?(jdescottes)

Reproduced in ESR 78

The issue comes from the PageStyleActor. This DevTools actor holds a WeakMap of all stylesheets added for the "add new rule" feature. This WeakMap is keyed by document (we add one stylesheet per document). But when we delete the stylesheet, nothing will cleanup the WeakMap.

Depending whether we use the watcher or not, this WeakMap is queried in different spots:

Instead of only querying the WeakMap in those spots, we could also check if the stylesheet is still in the document, by calling isConnected. I don't think anything else would be needed here.

We will have to add a test, which is probably going to make this a bit too complicated for a good-first-bug. We could start with something similar to one of the browser_rules_add-rule* tests. The steps should more or less be:

  • use add new rule and check the rule is applied
  • use SpecialPowers.spawn to delete the style element from the content page
  • use add new rule again and check the rule is applied

Setting myself as mentor, but note I will be away between Oct 25th and Nov 2nd

Mentor: jdescottes
Flags: needinfo?(jdescottes)

Hello,

I'm willing to work on this bug if nobody is assigned yet.

Thank you,
Colin

Hello Colin, thanks for offering help :)
The bug is now yours. Let me know if you have any question

Assignee: nobody → colin.cazabet
Status: NEW → ASSIGNED

Hello Julian,

I fixed the problem but I have some questions regarding the testing part.
I never used "SpecialPowers" and I cannot find the documentation on it, could you try to explain me what SpecialPowers.spawn does ?

Thank you

Hello Julian,

I found how to use "SpecialPowers" by looking on searchfox, I think I fixed the bug. I also wrote some tests

Thank you

Hi Colin, sorry I missed your questions :/

Glad you managed to use SpecialPowers!
For info you can use the "Request information from" checkbox below to make sure we receive a notification for your question.

Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/afa784e31b58 [devtools] Fix 'Add new rule' button when style node is deleted or moved. r=jdescottes

https://hg.mozilla.org/mozilla-central/rev/afa784e31b58

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
status-firefox97: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 97 Branch
QA Whiteboard: [qa-97b-p2]

I was able to reproduce the issue on Win10x64 using build 95.0a1(20211018214442) and steps from description.
Verified as fixed on Win10/Mac 10.13/Ubuntu 20.4 using builds 98.0a1(20220131094559) and 97.0b9 (20220127193706).

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-97b-p2]
status-firefox97: fixed → verified
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: