Closed
Bug 1241046
Opened 8 years ago
Closed 8 years ago
Editing an unmatched rule selector to match the current element doesn't do the right thing
Categories
(DevTools :: Inspector: Rules, defect, P2)
DevTools
Inspector: Rules
Tracking
(firefox48 fixed)
RESOLVED
FIXED
Firefox 48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: pbro, Assigned: tromey)
Details
(Whiteboard: [btpp-fix-later])
Attachments
(1 file, 1 obsolete file)
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•8 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•8 years ago
|
||
This seems to do the right thing. I'll write a test next week sometime.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → ttromey
Status: NEW → ASSIGNED
Reporter | ||
Comment 3•8 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•8 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•8 years ago
|
||
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•8 years ago
|
Attachment #8711159 -
Attachment is obsolete: true
Reporter | ||
Updated•8 years ago
|
Attachment #8740614 -
Flags: review?(pbrosset) → review+
Reporter | ||
Comment 6•8 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•8 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•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0c67560f23e8ad57deb628c8bf37d33ca3a79a4
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6a623b9d4599
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•