Closed
Bug 1327767
Opened 7 years ago
Closed 7 years ago
Ruleview displays wrong information about inherited rules: reports 1 element instead of many
Categories
(DevTools :: Inspector: Rules, defect, P2)
DevTools
Inspector: Rules
Tracking
(firefox53 verified)
VERIFIED
FIXED
Firefox 53
Tracking | Status | |
---|---|---|
firefox53 | --- | verified |
People
(Reporter: arni2033, Assigned: sblin)
References
Details
Attachments
(1 file, 2 obsolete files)
2.76 KB,
patch
|
gl
:
review+
|
Details | Diff | Splinter Review |
>>> My Info: Win7_64, Nightly 49, 32bit, ID 20160526082509
STR_1:
1. Open url data:text/html,<div p1>A<div p2>B<a>Color<style>[p1]{color:red}[p2]{color:black}
2. Open inspector -> ruleview, inspect <a> element
AR: Ruleview shows section "Inherited from div", and shows rules inherited from several div elements
ER: Ruleview shouldn't lie. Either X or Y
X) Ruleview should show 2 different sections for each <div> element
Y) The section should say "Inherited from several div elements"
Updated•7 years ago
|
Component: Untriaged → Developer Tools: Inspector
Priority: -- → P2
Component: Developer Tools: Inspector → Developer Tools: CSS Rules Inspector
Comment 1•7 years ago
|
||
If you change <div p1> to <span p1> you can see that in this case the rule view shows "Inherited from div" and "Inherited from span", which does seem more clear.
Assignee | ||
Comment 2•7 years ago
|
||
Attachment #8824151 -
Flags: review?(pbrosset)
Comment 3•7 years ago
|
||
Comment on attachment 8824151 [details] [diff] [review] 1327767.patch Sorry Gabriel, but I'm swamped in review/ni requests since I came back from PTO and I need to delegate a few ones if I want to get anything done. Sorry Sébastien for the delay. Your solution looks good to me. I had investigated this problem too and found something similar. Thanks for the test too!
Attachment #8824151 -
Flags: review?(pbrosset) → review?(gl)
Comment 4•7 years ago
|
||
Comment on attachment 8824151 [details] [diff] [review] 1327767.patch Review of attachment 8824151 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for this patch Sébastien. This is good to go once we make the changes. ::: devtools/client/inspector/rules/test/browser_rules_inherited-properties_04.js @@ +5,5 @@ > +"use strict"; > + > +// Check that inline inherited properties appear in the nested element. > + > +const TEST_URI = `<div style="cursor:pointer">A<div style="cursor:pointer">B<a>Cursor`; We should add the closing brackets for this TEST_URI. It would also be clearer if we change the second div to a span, and then check that it is "Inherited from span".
Attachment #8824151 -
Flags: review?(gl) → review+
Updated•7 years ago
|
Assignee: nobody → amarok
Updated•7 years ago
|
Status: NEW → ASSIGNED
Comment 5•7 years ago
|
||
Also, we will need to update the commit message to the following: Bug 1327767 - Show multiples sections for rules inherited from several elements of the same type. r=gl
Assignee | ||
Comment 6•7 years ago
|
||
Cool. I will do this tomorrow.
Assignee | ||
Comment 7•7 years ago
|
||
I made the changes.
> It would also be clearer if we change the second div to a span, and then check that it is "Inherited from span"
But this patch test the creation of two sections if we have 2 div or 2 span, not one div and one span. So I don't think we need to change this.
Have a nice day,
Attachment #8824151 -
Attachment is obsolete: true
Attachment #8825836 -
Flags: review?(gl)
Comment 8•7 years ago
|
||
Comment on attachment 8825836 [details] [diff] [review] bug_1327767.patch Review of attachment 8825836 [details] [diff] [review]: ----------------------------------------------------------------- Hi Sébastien, Thanks for getting to this so quickly. We still also need to update the commit message in your patch file to: Bug 1327767 - Show multiples sections for rules inherited from several elements of the same type. r=gl I can also help you push this when it is ready with the changes I mentioned. ::: devtools/client/inspector/rules/test/browser_rules_inherited-properties_04.js @@ +3,5 @@ > + http://creativecommons.org/publicdomain/zero/1.0/ */ > + > +"use strict"; > + > +// Check that inline inherited properties appear in the nested element. The test comment should also read as follows: // Tests that different inherited properties sections are created for rules inherited from several elements of the same type. @@ +10,5 @@ > + A > + <div style="cursor:pointer"> > + B<a>Cursor</a> > + </div> > + </div>`; We want to be consistent with how we do multiline strings as the other tests. I would like this to be formatted as the following: const TEST_URI = ` <div style="cursor:pointer"> A <div style="cursor:pointer"> B<a>Cursor</a> </div> </div> `;
Attachment #8825836 -
Flags: review?(gl) → review+
Assignee | ||
Comment 9•7 years ago
|
||
Attachment #8825836 -
Attachment is obsolete: true
Attachment #8825968 -
Flags: review?(gl)
Comment 10•7 years ago
|
||
Comment on attachment 8825968 [details] [diff] [review] bug_1327767.patch Review of attachment 8825968 [details] [diff] [review]: ----------------------------------------------------------------- SHIPIT Sébastien! Did you want me to push this for you?
Attachment #8825968 -
Flags: review?(gl) → review+
Assignee | ||
Comment 11•7 years ago
|
||
I think I don't have the right to push this.
Comment 12•7 years ago
|
||
Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7166b68a598e Show multiples sections for rules inherited from several elements of the same type. r=gl
Comment 13•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7166b68a598e
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Updated•7 years ago
|
Flags: qe-verify+
Comment 14•7 years ago
|
||
Successfully reproduce this bug on Nightly 49.0a1 (2016-05-26) (Build ID: 20160526030223) by the following Comment 0's instruction! This Bug is now Verified as Fixed on Latest Firefox Beta 53.0b1 (64-bit) Build ID: 20170307144748 User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0 OS: Linux 4.8.0-39-generic; Ubuntu 16.04.2 (64 Bit)
QA Whiteboard: [bugday-20170308]
Comment 15•7 years ago
|
||
Reproduced this bug with 2017-01-01 53.0a1 build. Verified using Firefox 53 beta 1 under Win 10 64-bit and Mac OS X 10.11 that ruleview now shows two different sections for each <div> element. Marking as verified considering this and comment 14.
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•