Support ::marker pseudo-element in inactiveCSS
Categories
(DevTools :: Inspector: Rules, defect, P3)
Tracking
(Not tracked)
People
(Reporter: pbro, Unassigned)
References
(Blocks 2 open bugs)
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
- open
data:text/html,<style>li::before{content:"";position:relative;top:10px;}li::marker{content:"";position:relative;top:10px;}</style><ul><li>test
- open the inspector and select the
::marker
pseudo-element
Expected: the top
property should not be greyed-out
Actual: the top
property is marked as being inactive
This comes from the fact that ::marker
doesn't seem to be supported well in InactiveCSS.
As a comparison, try the ::before
pseudo-element in that page, you'll see things work well there.
Also, the same happens for other types of warnings, it's not only due to the top
property.
Reporter | ||
Comment 1•5 years ago
|
||
So far, investigation shows that it could be coming from this line:
return node.ownerGlobal.getComputedStyle(bindingElement, pseudo);
This is where we get the computed style for the currently selected element in the inspector.
node.ownerGlobal
is just the container window object.
bindingElement
in this case is the <li>
element.
pseudo
is the string :marker
.
And, sure enough, if I run the equivalent line of code on the test page, using the console:
window.getComputedStyle($("li"), ":marker");
Then I get "static" instead of "relative" as I was expecting.
So it could be a problem (or limitation) with how :marker
was implemented in Firefox.
Comment 2•5 years ago
|
||
Mats, I hope you can help here. Are we defining the ::marker pseudo element correctly in nsCSSPseudoElementList.h? Though not defined in CSS2, it seems that the element can be selected with the single-colon syntax. Should it have the CSS_PSEUDO_ELEMENT_IS_CSS2 flag? Likewise, the marker box seemingly does contain elements according to the spec, but we don't give it the CSS_PSEUDO_ELEMENT_CONTAINS_ELEMENTS flag. Are these flags set correctly?
Comment 3•5 years ago
|
||
(In reply to Brad Werth [:bradwerth] from comment #2)
Though not defined in CSS2, it seems that the element can be selected with the single-colon syntax. Should it have the CSS_PSEUDO_ELEMENT_IS_CSS2 flag?
No, it shouldn't support single-colon syntax, and it doesn't.
Likewise, the marker box seemingly [does contain elements according to the spec][1], but we don't give it the CSS_PSEUDO_ELEMENT_CONTAINS_ELEMENTS flag.
Well, it contains anonymous content only. An author can't inject a <div>
or whatever into it. The CSS_PSEUDO_ELEMENT_CONTAINS_ELEMENTS is for ::first-line/letter
which wraps DOM content, including elements.
Are these flags set correctly?
Yes, I believe they are.
Comment 4•5 years ago
|
||
(In reply to Patrick Brosset <:pbro> from comment #1)
And, sure enough, if I run the equivalent line of code on the test page, using the console:
window.getComputedStyle($("li"), ":marker");
Then I get "static" instead of "relative" as I was expecting.
Two problems: first you need to use "::marker" since ":marker" just behaves as any other unknown pseudo like ":foo"; second, we have disabled support for many properties on ::marker
because the CSS spec isn't finished yet. This is controlled by the layout.css.marker.restricted
pref. If you toggle that on and use two colons you should get relative
.
I would expect that pref to be enabled by default soon-ish, so I wouldn't spend time on trying to figure out which properties apply in the current situation.
Comment 5•5 years ago
|
||
Thanks, Mats. That's very helpful. So, to summarize, this is working as intended. ::marker is restricted in which properties it allows, and "top" is not on that list. As far as devtools, we can consider whether we want to customize the message in this situation. Patrick, would you rather we close this as INVALID, or do we keep it open for a more specific tooltip/message?
Reporter | ||
Comment 6•5 years ago
|
||
Thank you both, this is very helpful.
My hunch for the best approach to take here is: let's bail out if the element is the ::marker pseudo-element and not attempt to show warnings. Because the list of properties supported is so tiny, and because we can't access them yet until layout.css.marker.restricted
is ON, I think it's safer to just ignore this pseudo-element.
If we wanted to support ::marker, we'd have to do it on a case by case basis in our inactivecss rules engine, and would end up complexifying it quite substantially for not much reasons.
So, we should keep this bug, but use it to ignore ::marker pseudo-elements. We should also add a comment in the code explaining why, as things may change in the future and we might want to revisit this decision.
Happy to hear other thoughts on this.
Reporter | ||
Comment 7•5 years ago
|
||
Reporter | ||
Comment 8•5 years ago
|
||
I left the above patch to give an idea of what I have in mind. Feel free to pick up from here and land this (it would be good to add a test).
Comment 9•5 years ago
|
||
The priority flag is not set for this bug.
:gl, could you have a look please?
For more information, please visit auto_nag documentation.
Updated•5 years ago
|
Updated•2 years ago
|
Updated•1 year ago
|
Description
•