Open Bug 2016841 Opened 2 months ago Updated 1 month ago

Add UI for media state pseudo-classes

Categories

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

enhancement

Tracking

(Not tracked)

People

(Reporter: sebo, Unassigned, NeedInfo)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-needed)

Attachments

(2 files)

Bug 1707584 implemented a bunch of media state pseudo-classes, namely :playing, :paused, :seeking, :buffering, :stalled, :muted and :volume-locked.

The DevTools should support emulating all those pseudo-classes to allow tweaking their styles.

Note that those pseudo-classes are part of the Interop 2026 project. So having support for them in the DevTools when they are shipped might gain some attention. Therefore, I think they should get some priority.

Sebastian

I quickly wanted to see if it's only about adding the right classes in the right spots
You can quickly show them in the pseudo class panel with:

diff --git a/devtools/shared/css/constants.js b/devtools/shared/css/constants.js
index cc8748c5536f..7f99045c0402 100644
--- a/devtools/shared/css/constants.js
+++ b/devtools/shared/css/constants.js
@@ -35,4 +35,11 @@ exports.PSEUDO_CLASSES = [
   ":focus-within",
   ":visited",
   ":target",
+  ":playing",
+  ":paused",
+  ":seeking",
+  ":buffering",
+  ":stalled",
+  ":muted",
+  ":volume-locked",
 ];

and it does work for some cases (e.g. :stalled)
When trying to click on the :playing checkbox, it automatically checks the :paused one for some reason
Trying to click on the :volume-locked one doesn't work (the checkbox gets unchecked directly)
So we'd need to figure this out.
We should also probably put those in a specific section, and only having them visible when a HTMLMediaElement is selected (so <audio> or <video>)
We also need to handle the "exclusiveness" of those (e.g. if you check :paused, you :playing should be toggled)
I also wonder if enabling those should actually impact the element itself. It could be weird having the video actually playing and the :paused pseudo being applied? I'm not sure about this one though

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #1)

Trying to click on the :volume-locked one doesn't work (the checkbox gets unchecked directly)

Maybe that's related to bug 2013371?

We should also probably put those in a specific section, and only having them visible when a HTMLMediaElement is selected (so <audio> or <video>)

Yes, with more and more pseudo-class toggles getting added, the panel slowly gets confusing. The Chrome Devtools put them in a togglable "Force specific element state" section, which sounds somewhat reasonable.

We also need to handle the "exclusiveness" of those (e.g. if you check :paused, you :playing should be toggled)

Agree. Just note that other pseudo-classes like :valid/:invalid, :read-only/read-write, or :in-range/:out-of-range also have such an exclusiveness. So the functionality should work for multiple individual pairs of pseudo-classes.

I also wonder if enabling those should actually impact the element itself. It could be weird having the video actually playing and the :paused pseudo being applied? I'm not sure about this one though

I also gave that some thought and believe it's rather disrupting the debugging process when toggling the pseudo-class actually has an impact on the playing state of the element. Toggling them should really just apply the styles. This is also consistent with the existing pseudo-class toggles like :active, which does not influence the actual active element, so e.g. document.activeElement still returns the real active element.

Sebastian

(In reply to Sebastian Zartner [:sebo] from comment #2)

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #1)

We should also probably put those in a specific section, and only having them visible when a HTMLMediaElement is selected (so <audio> or <video>)

Yes, with more and more pseudo-class toggles getting added, the panel slowly gets confusing. The Chrome Devtools put them in a togglable "Force specific element state" section, which sounds somewhat reasonable.

Heads up, I've added one in the patch for bug 2014442. Feedback welcome! ☺️

Sebastian

Depends on: 2014442
Priority: -- → P3

note that we should make the list of element-specific pseudo-class sorted alphabetically

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #1)

When trying to click on the :playing checkbox, it automatically checks the :paused one for some reason

Alastor, maybe you as the implementer of those pseudo-classes can help out here. I didn't did deeper yet, though the logic for locking the pseudo-classes is generally the same for all pseudo-classes. (Entry point is the InspectorUtils.addPseudoClassLock() method.)

Sebastian

Flags: needinfo?(alwu)

(In reply to Sebastian Zartner [:sebo] from comment #5)

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #1)

When trying to click on the :playing checkbox, it automatically checks the :paused one for some reason

Alastor, maybe you as the implementer of those pseudo-classes can help out here. I didn't did deeper yet, though the logic for locking the pseudo-classes is generally the same for all pseudo-classes. (Entry point is the InspectorUtils.addPseudoClassLock() method.)

Sebastian

:playing and :paused are mutually exclusive, so only one can be active at a time. We implement it in just one bit by design to prevent both of them are active.

Flags: needinfo?(alwu)

Ok, then InspectorUtils.cpp probably needs to be adjusted to be able to handle those single-bit element states. I'm just wondering whether that then works as expected, because with two distinct bits you can also toggle them individually. I guess with only one bit one of the locks will always be active. Though I still have to play a bit with the code.
Nicolas, I am wondering whether you could also have a look at this and provide some input.

Sebastian

Flags: needinfo?(nchevobbe)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: