Closed Bug 1615473 Opened 4 years ago Closed 4 years ago

Support ::marker for the animation inspector

Categories

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

Product:
DevTools
Component:
Inspector: Animations
Type:
enhancement

Tracking

(firefox75 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 75
Tracking Flags:
Tracking Status
firefox75 --- fixed

People

(Reporter: boris, Assigned: daisuke)

References

(Depends on 1 open bug)

Details

Attachments

(4 files)

Bug 1615473: Support animation on ::marker pseudo element.
(deleted), text/x-phabricator-request
Details
Bug 1615473: Add a test case for animation on ::marker pseudo element.
(deleted), text/x-phabricator-request
Details
marker.html
(deleted), text/html
Details
::marker screenshot
(deleted), image/png
Details

Now we support ::marker (Bug 1538618) in web-animations, and will also support ::first-line and ::first-letter (Bug 1615469) later, so perhaps we have to update animation.js and other test cases (e.g. browser_animation_pseudo-element.js).

Assignee: nobody → daisuke
Status: NEW → ASSIGNED

BTW, ::first-line, and ::first-letter is not supported for animation yet, but it's still worth adding these cases in the inspector.

I checked to fix ::first-letter as well, but it seems that the way to implement differs from ::before etc.
In ::before pseudo element, we generate a node of _moz_generated_content_before, but it seems that not do for ::first-letter.
https://searchfox.org/mozilla-central/search?q=_moz_generated_content_&case=false&regexp=false&path=
And also, we look for the target node using DocumentWalker, it seems that can't find that as far as I tried.
https://searchfox.org/mozilla-central/source/devtools/server/actors/inspector/document-walker.js
Thus, as we need to investigate more, please let me fix ::first-letter and ::first-line in another bug.

Attached file marker.html (deleted) —

Hi Boris!
Please let me confirm some behaviors of ::marker animation.

  1. CSS Animation:
    Please see the .css-animation element. As we talked at slack, it seems CSS Animation for ::marker does not run. As you said, this may be a regression.
  2. Animatable property for ::marker:
    Please see the .not-animation element. It seems that opacity is not applied regardless we set opacity 0.1. On the other hand, please see .script-animation element. I added an animation that changes the opacity. And the opacity is changed by the animation.
    According to the spec, it seems that we can apply very limited properties but not opacity.
    https://drafts.csswg.org/css-pseudo-4/#marker-pseudo
    So, I'd like to confirm whether this behavior is correct or not.

Thanks!

Flags: needinfo?(boris.chiou)
Attached image ::marker screenshot (deleted) —

Marker seems to already work for me somehow.

Not sure why CSS animations aren't working though, since this test suggests they should.

Also, we technically shouldn't support opacity animations on ::marker elements since they're not a currently a supported property.

(Oh, just noticed Daisuke's second point in comment #5 which says the same thing about opacity.)

I investigated a bit about CSS Animation for ::marker.
In order to run this animation, it seems that we should turn on layout.css.marker.restricted pref.

(In reply to Daisuke Akatsuka (:daisuke) from comment #9)

I investigated a bit about CSS Animation for ::marker.
In order to run this animation, it seems that we should turn on layout.css.marker.restricted pref.

Sorry, we should turn off layout.css.marker.restricted pref.
And the default value is true.

Pushed by dakatsuka.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4df822e7d09c
Support animation on ::marker pseudo element. r=pbro
https://hg.mozilla.org/integration/autoland/rev/51efc4b931f7
Add a test case for animation on ::marker pseudo element. r=pbro
Summary: Support ::marker, ::first-line, and ::first-letter for the animation inspector → Support ::first-line, and ::first-letter for the animation inspector
Summary: Support ::first-line, and ::first-letter for the animation inspector → Support ::marker for the animation inspector

https://hg.mozilla.org/mozilla-central/rev/4df822e7d09c
https://hg.mozilla.org/mozilla-central/rev/51efc4b931f7

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox75: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 75

(In reply to Daisuke Akatsuka (:daisuke) from comment #10)

Sorry, we should turn off layout.css.marker.restricted pref.
And the default value is true.

Great. Thanks for this investigation.

Please see the .not-animation element. It seems that opacity is not applied regardless we set opacity 0.1. On the other hand, please see .script-animation element. I added an animation that changes the opacity. And the opacity is changed by the animation.
According to the spec, it seems that we can apply very limited properties but not opacity.
https://drafts.csswg.org/css-pseudo-4/#marker-pseudo
So, I'd like to confirm whether this behavior is correct or not.

I believe we shouldn't apply opacity on ::marker, as Brian mentioned (and css-pseudo4 spec you provided). If its animation still applies, it should be a bug I think.

Sorry for the late reply.

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

Attachment

General

Creator:
Created:
Updated:
Size: