Bug 1210796 - Part 11: Disable clicking on keyframes because we can’t calculate correct current time since the animation detail handles the animation progress not time. r=pbro
MozReview-Commit-ID: JhXNREIyH9X
--- a/devtools/client/animationinspector/components/animation-details.js
+++ b/devtools/client/animationinspector/components/animation-details.js
@@ -21,18 +21,16 @@ const L10N =
* This includes information about timing, easing, keyframes, animated
* properties.
*
* @param {Object} serverTraits The list of server-side capabilities.
*/
function AnimationDetails(serverTraits) {
EventEmitter.decorate(this);
- this.onFrameSelected = this.onFrameSelected.bind(this);
-
this.keyframeComponents = [];
this.serverTraits = serverTraits;
}
exports.AnimationDetails = AnimationDetails;
AnimationDetails.prototype = {
// These are part of frame objects but are not animated properties. This
@@ -48,17 +46,16 @@ AnimationDetails.prototype = {
this.unrender();
this.containerEl = null;
this.serverTraits = null;
this.progressIndicatorEl = null;
},
unrender: function () {
for (let component of this.keyframeComponents) {
- component.off("frame-selected", this.onFrameSelected);
component.destroy();
}
this.keyframeComponents = [];
while (this.containerEl.firstChild) {
this.containerEl.firstChild.remove();
}
},
@@ -202,21 +199,16 @@ AnimationDetails.prototype = {
this.dummyAnimation =
new this.win.Animation(new this.win.KeyframeEffect(null, null, timing), null);
// Useful for tests to know when rendering of all animation detail UIs
// have been completed.
this.emit("animation-detail-rendering-completed");
}),
- onFrameSelected: function (e, args) {
- // Relay the event up, it's needed in parents too.
- this.emit(e, args);
- },
-
renderAnimatedPropertiesHeader: function () {
// Add animated property header.
const headerEl = createNode({
parent: this.containerEl,
attributes: { "class": "animated-properties-header" }
});
// Add progress tick container.
@@ -289,17 +281,16 @@ AnimationDetails.prototype = {
let keyframesComponent = new Keyframes();
keyframesComponent.init(framesEl);
keyframesComponent.render({
keyframes: this.tracks[propertyName],
propertyName: propertyName,
animation: this.animation,
animationType: animationTypes[propertyName]
});
- keyframesComponent.on("frame-selected", this.onFrameSelected);
this.keyframeComponents.push(keyframesComponent);
}
},
renderProgressIndicator: function () {
// The wrapper represents the area which the indicator is displayable.
const progressIndicatorWrapperEl = createNode({
parent: this.containerEl,
--- a/devtools/client/animationinspector/components/animation-timeline.js
+++ b/devtools/client/animationinspector/components/animation-timeline.js
@@ -53,17 +53,16 @@ function AnimationsTimeline(inspector, s
this.onAnimationStateChanged = this.onAnimationStateChanged.bind(this);
this.onScrubberMouseDown = this.onScrubberMouseDown.bind(this);
this.onScrubberMouseUp = this.onScrubberMouseUp.bind(this);
this.onScrubberMouseOut = this.onScrubberMouseOut.bind(this);
this.onScrubberMouseMove = this.onScrubberMouseMove.bind(this);
this.onAnimationSelected = this.onAnimationSelected.bind(this);
this.onWindowResize = this.onWindowResize.bind(this);
- this.onFrameSelected = this.onFrameSelected.bind(this);
this.onTimelineDataChanged = this.onTimelineDataChanged.bind(this);
EventEmitter.decorate(this);
}
exports.AnimationsTimeline = AnimationsTimeline;
AnimationsTimeline.prototype = {
@@ -267,17 +266,16 @@ AnimationsTimeline.prototype = {
unrender: function () {
for (let animation of this.animations) {
animation.off("changed", this.onAnimationStateChanged);
}
this.stopAnimatingScrubber();
TimeScale.reset();
this.destroySubComponents("targetNodes");
this.destroySubComponents("timeBlocks");
- this.details.off("frame-selected", this.onFrameSelected);
this.details.unrender();
this.animationsEl.innerHTML = "";
this.off("timeline-data-changed", this.onTimelineDataChanged);
},
onWindowResize: function () {
// Don't do anything if the root element has a width of 0
if (this.rootWrapperEl.offsetWidth === 0) {
@@ -329,22 +327,18 @@ AnimationsTimeline.prototype = {
this.animationDetailEl.dataset.defaultDisplayStyle;
yield this.details.render(animation);
this.onTimelineDataChanged(null, { time: this.currentTime || 0 });
this.animationAnimationNameEl.textContent = getFormattedAnimationTitle(animation);
this.emit("animation-selected", animation);
}),
/**
- * When a frame gets selected, move the scrubber to the corresponding position
+ * When move the scrubber to the corresponding position
*/
- onFrameSelected: function (e, {x}) {
- this.moveScrubberTo(x, true);
- },
-
onScrubberMouseDown: function (e) {
this.moveScrubberTo(e.pageX);
this.win.addEventListener("mouseup", this.onScrubberMouseUp);
this.win.addEventListener("mouseout", this.onScrubberMouseOut);
this.win.addEventListener("mousemove", this.onScrubberMouseMove);
// Prevent text selection while dragging.
e.preventDefault();
@@ -468,17 +462,16 @@ AnimationsTimeline.prototype = {
// Draw the animation time block.
let timeBlock = new AnimationTimeBlock();
timeBlock.init(timeBlockEl);
timeBlock.render(animation);
this.timeBlocks.push(timeBlock);
timeBlock.on("selected", this.onAnimationSelected);
}
- this.details.on("frame-selected", this.onFrameSelected);
// Use the document's current time to position the scrubber (if the server
// doesn't provide it, hide the scrubber entirely).
// Note that because the currentTime was sent via the protocol, some time
// may have gone by since then, and so the scrubber might be a bit late.
if (!documentCurrentTime) {
this.scrubberEl.style.display = "none";
} else {
--- a/devtools/client/animationinspector/components/keyframes.js
+++ b/devtools/client/animationinspector/components/keyframes.js
@@ -16,35 +16,31 @@ const {ProgressGraphHelper, appendPathEl
let LINEAR_GRADIENT_ID_COUNTER = 0;
/**
* UI component responsible for displaying a list of keyframes.
* Also, shows a graphical graph for the animation progress of one iteration.
*/
function Keyframes() {
EventEmitter.decorate(this);
- this.onClick = this.onClick.bind(this);
}
exports.Keyframes = Keyframes;
Keyframes.prototype = {
init: function (containerEl) {
this.containerEl = containerEl;
this.keyframesEl = createNode({
parent: this.containerEl,
attributes: {"class": "keyframes"}
});
-
- this.containerEl.addEventListener("click", this.onClick);
},
destroy: function () {
- this.containerEl.removeEventListener("click", this.onClick);
this.keyframesEl.remove();
this.containerEl = this.keyframesEl = this.animation = null;
},
render: function ({keyframes, propertyName, animation, animationType}) {
this.keyframes = keyframes;
this.propertyName = propertyName;
this.animation = animation;
@@ -94,32 +90,16 @@ Keyframes.prototype = {
"class": "frame",
"style": `left:${frame.offset * 100}%;`,
"data-offset": frame.offset,
"data-property": propertyName,
"title": frame.value
}
});
}
- },
-
- onClick: function (e) {
- // If the click happened on a frame, tell our parent about it.
- if (!e.target.classList.contains("frame")) {
- return;
- }
-
- e.stopPropagation();
- this.emit("frame-selected", {
- animation: this.animation,
- propertyName: this.propertyName,
- offset: parseFloat(e.target.dataset.offset),
- value: e.target.getAttribute("title"),
- x: e.target.offsetLeft + e.target.closest(".frames").offsetLeft
- });
}
};
/**
* Render a graph representing the progress of the animation over one iteration.
* @param {Element} parentEl - Parent element of this appended path element.
* @param {Number} duration - Duration of one iteration.
* @param {Number} minSegmentDuration - Minimum segment duration.
--- a/devtools/client/animationinspector/test/browser.ini
+++ b/devtools/client/animationinspector/test/browser.ini
@@ -22,17 +22,16 @@ support-files =
!/devtools/client/shared/test/test-actor-registry.js
!/devtools/client/shared/test/test-actor.js
[browser_animation_animated_properties_displayed.js]
[browser_animation_click_selects_animation.js]
[browser_animation_controller_exposes_document_currentTime.js]
skip-if = os == "linux" && !debug # Bug 1234567
[browser_animation_empty_on_invalid_nodes.js]
-[browser_animation_keyframe_click_to_set_time.js]
[browser_animation_keyframe_markers.js]
[browser_animation_mutations_with_same_names.js]
[browser_animation_panel_exists.js]
[browser_animation_participate_in_inspector_update.js]
[browser_animation_playerFronts_are_refreshed.js]
[browser_animation_playerWidgets_appear_on_panel_init.js]
[browser_animation_playerWidgets_target_nodes.js]
[browser_animation_pseudo_elements.js]
deleted file mode 100644
--- a/devtools/client/animationinspector/test/browser_animation_keyframe_click_to_set_time.js
+++ /dev/null
@@ -1,49 +0,0 @@
-/* vim: set ts=2 et sw=2 tw=80: */
-/* Any copyright is dedicated to the Public Domain.
- http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-// Test that animated properties' keyframes can be clicked, and that doing so
-// sets the current time in the timeline.
-
-add_task(function* () {
- yield addTab(URL_ROOT + "doc_keyframes.html");
- let {panel} = yield openAnimationInspector();
- let timeline = panel.animationsTimelineComponent;
- let {scrubberEl} = timeline;
-
- // XXX: The scrollbar is placed in the timeline in such a way that it causes
- // the animations to be slightly offset with the header when it appears.
- // So for now, let's hide the scrollbar. Bug 1229340 should fix this.
- timeline.animationsEl.style.overflow = "hidden";
-
- info("Click on the first keyframe of the first animated property");
- yield clickKeyframe(panel, "background-color", 0);
-
- info("Make sure the scrubber stopped moving and is at the right position");
- yield assertScrubberMoving(panel, false);
- checkScrubberPos(scrubberEl, 0);
-
- info("Click on a keyframe in the middle");
- yield clickKeyframe(panel, "transform", 2);
-
- info("Make sure the scrubber is at the right position");
- checkScrubberPos(scrubberEl, 50);
-});
-
-function* clickKeyframe(panel, property, index) {
- let keyframeComponent = getKeyframeComponent(panel, property);
- let keyframeEl = getKeyframeEl(panel, property, index);
-
- let onSelect = keyframeComponent.once("frame-selected");
- EventUtils.sendMouseEvent({type: "click"}, keyframeEl,
- keyframeEl.ownerDocument.defaultView);
- yield onSelect;
-}
-
-function checkScrubberPos(scrubberEl, pos) {
- let newPos = Math.round(parseFloat(scrubberEl.style.left));
- let expectedPos = Math.round(pos);
- is(newPos, expectedPos, `The scrubber is at ${pos}%`);
-}
--- a/devtools/client/themes/animationinspector.css
+++ b/devtools/client/themes/animationinspector.css
@@ -619,17 +619,16 @@ body {
}
.keyframes .frame {
position: absolute;
top: 50%;
width: 0;
height: 0;
background-color: inherit;
- cursor: pointer;
}
.keyframes .frame::before {
content: "";
display: block;
transform:
translateX(calc(var(--keyframes-marker-size) * -.5))
/* The extra pixel on the Y axis is so that markers are centered on the