Editing an unmatched rule selector to match the current element doesn't do the right thing

RESOLVED FIXED in Firefox 48

Status

()

Firefox
Developer Tools: CSS Rules Inspector
P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: pbro, Assigned: tromey)

Tracking

unspecified
Firefox 48
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

(Whiteboard: [btpp-fix-later])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
STR:

- Go to about:home
- Open the inspector with the rule-view visible
- In the rule-view, click on the 'html' rule selector and change it to 'span'
- This makes the rule not match the current element, so it gets greyed-out as you would expect
- Now edit again the same select and change it back to 'html'

=> Expected: the rule now matches the current element and should go back to its original styling (not greyed-out).
=> Actual: the rule's styling remains unchanged.

The actual CSS rule is changed correctly on the content page though.
(Assignee)

Comment 1

2 years ago
The problem occurs here:

https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/styles.js#1637

For example if the node is the <body> and value is "html":

>> node.rawNode
<body dir="ltr" narrow="true">
>> node.rawNode.matches(value)
false
(Assignee)

Comment 2

2 years ago
Created attachment 8711159 [details] [diff] [review]
find inherited entry in modifySelector2

This seems to do the right thing.
I'll write a test next week sometime.
(Assignee)

Updated

2 years ago
Assignee: nobody → ttromey
Status: NEW → ASSIGNED
(Reporter)

Comment 3

2 years ago
Triaging this as a P2 as quite important but not crashing the tools either.
Tom, it looks like you had uploaded a patch that worked a couple of months ago. Do you still intend to work on this?
Flags: needinfo?(ttromey)
Priority: -- → P2
Whiteboard: [btpp-fix-later]
(Assignee)

Comment 4

2 years ago
Yeah.  I'm sorry about the delay here, I got sucked in by network throttling.
I'll write the test this week.
Flags: needinfo?(ttromey)
(Assignee)

Comment 5

2 years ago
Created attachment 8740614 [details]
MozReview Request: Bug 1241046 - find inherited entry in modifySelector2; r=pbro

Review commit: https://reviewboard.mozilla.org/r/45837/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/45837/
Attachment #8740614 - Flags: review?(pbrosset)
(Assignee)

Updated

2 years ago
Attachment #8711159 - Attachment is obsolete: true
(Reporter)

Updated

2 years ago
Attachment #8740614 - Flags: review?(pbrosset) → review+
(Reporter)

Comment 6

2 years ago
Comment on attachment 8740614 [details]
MozReview Request: Bug 1241046 - find inherited entry in modifySelector2; r=pbro

https://reviewboard.mozilla.org/r/45837/#review42915

Looks good to me. Thanks for the very clear test.

::: devtools/client/inspector/rules/test/browser_rules_edit-selector_08.js:29
(Diff revision 1)
> +
> +  info("Selecting the test element");
> +  yield selectNode("#testid", inspector);
> +
> +  let idRuleEditor = getRuleViewRuleEditor(view, 2);
> +  info("FOCUS WAT " + idRuleEditor.selectorText);

This looks like a log you forgot to remove.
(Assignee)

Comment 7

2 years ago
Comment on attachment 8740614 [details]
MozReview Request: Bug 1241046 - find inherited entry in modifySelector2; r=pbro

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45837/diff/1-2/
Attachment #8740614 - Attachment description: MozReview Request: Bug 1241046 - find inherited entry in modifySelector2; r?pbro → MozReview Request: Bug 1241046 - find inherited entry in modifySelector2; r=pbro
(Assignee)

Comment 8

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0c67560f23e8ad57deb628c8bf37d33ca3a79a4
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 9

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/6a623b9d4599
Keywords: checkin-needed

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6a623b9d4599
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
You need to log in before you can comment on or make changes to this bug.