diff --git a/devtools/client/inspector/rules/test/browser_rules_edit-property-cancel.js b/devtools/client/inspector/rules/test/browser_rules_edit-property-cancel.js index 443f8432f80c..477f58a623da 100644 --- a/devtools/client/inspector/rules/test/browser_rules_edit-property-cancel.js +++ b/devtools/client/inspector/rules/test/browser_rules_edit-property-cancel.js @@ -17,7 +17,7 @@ const TEST_URI = ` add_task(async function () { await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI)); - const { inspector, view } = await openRuleView(); + const { inspector, view } = await openRuleView({ overrideDebounce: false }); await selectNode("#testid", inspector); const ruleEditor = getRuleViewRuleEditor(view, 1); @@ -38,11 +38,28 @@ add_task(async function () { "#00F background color is set." ); - await focusEditableField(view, propEditor.valueSpan); + const editor = await focusEditableField(view, propEditor.valueSpan); + info("Delete input value"); const onValueDeleted = view.once("ruleview-changed"); - await sendKeysAndWaitForFocus(view, ruleEditor.element, ["DELETE", "ESCAPE"]); + EventUtils.sendKey("DELETE", view.styleWindow); await onValueDeleted; + is(editor.input.value, "", "value input is empty"); + + await waitFor(() => view.popup?.isOpen); + ok(true, "autocomplete popup opened"); + + info("Hide autocomplete popup"); + const onPopupClosed = once(view.popup, "popup-closed"); + EventUtils.sendKey("ESCAPE", view.styleWindow); + await onPopupClosed; + ok(true, "popup was closed"); + + info("Cancel edit with escape key"); + const onRuleViewChanged = view.once("ruleview-changed"); + EventUtils.sendKey("ESCAPE", view.styleWindow); + await onRuleViewChanged; + is( propEditor.valueSpan.textContent, "#00F", @@ -53,4 +70,10 @@ add_task(async function () { "rgb(0, 0, 255)", "#00F background color is set." ); + + is( + propEditor.warning.hidden, + true, + "warning icon is hidden after cancelling the edit" + ); }); diff --git a/devtools/client/inspector/rules/views/text-property-editor.js b/devtools/client/inspector/rules/views/text-property-editor.js index 78216f4901bd..e66c1e65d506 100644 --- a/devtools/client/inspector/rules/views/text-property-editor.js +++ b/devtools/client/inspector/rules/views/text-property-editor.js @@ -423,7 +423,19 @@ TextPropertyEditor.prototype = { start: this._onStartEditing, element: this.valueSpan, done: this._onValueDone, - destroy: this.update, + destroy: onValueDonePromise => { + const cb = this.update; + // The `done` callback is called before this `destroy` callback is. + // In _onValueDone, we might preview/set the property and we want to wait for + // that to be resolved before updating the view so all data are up to date (see Bug 1325145). + if ( + onValueDonePromise && + typeof onValueDonePromise.then === "function" + ) { + return onValueDonePromise.then(cb); + } + return cb(); + }, validate: this._onValidate, advanceChars: advanceValidate, contentType: InplaceEditor.CONTENT_TYPES.CSS_VALUE, @@ -1274,13 +1286,13 @@ TextPropertyEditor.prototype = { // If the value is not empty (or is an empty variable) and unchanged, // revert the property back to its original value and enabled or disabled state if ((value.trim() || isVariable) && isValueUnchanged) { - this.ruleEditor.rule.previewPropertyValue( + const onPropertySet = this.ruleEditor.rule.previewPropertyValue( this.prop, val.value, val.priority ); this.rule.setPropertyEnabled(this.prop, this.prop.enabled); - return; + return onPropertySet; } // Check if unit of value changed to add dragging feature @@ -1293,7 +1305,7 @@ TextPropertyEditor.prototype = { this.telemetry.recordEvent("edit_rule", "ruleview"); // First, set this property value (common case, only modified a property) - this.prop.setValue(val.value, val.priority); + const onPropertySet = this.prop.setValue(val.value, val.priority); if (!this.prop.enabled) { this.prop.setEnabled(true); @@ -1322,6 +1334,8 @@ TextPropertyEditor.prototype = { } }, 0); } + + return onPropertySet; }, /** diff --git a/devtools/client/inspector/test/shared-head.js b/devtools/client/inspector/test/shared-head.js index 6d46be3f4d31..06166b294376 100644 --- a/devtools/client/inspector/test/shared-head.js +++ b/devtools/client/inspector/test/shared-head.js @@ -81,19 +81,25 @@ var openInspectorSidebarTab = async function (id) { * Open the toolbox, with the inspector tool visible, and the rule-view * sidebar tab selected. * + * @param {Object} options + * @param {Boolean} options.overrideDebounce: Whether to replace the rule view debounce + * method with manual debounce (requires explicit calls to trigger the debounced calls). + * Defaults to true. * @return a promise that resolves when the inspector is ready and the rule view * is visible and ready */ -async function openRuleView() { +async function openRuleView({ overrideDebounce = true } = {}) { const { inspector, toolbox, highlighterTestFront } = await openInspector(); const ruleViewPanel = inspector.getPanel("ruleview"); await ruleViewPanel.readyPromise; const view = ruleViewPanel.view; - // Replace the view to use a custom debounce function that can be triggered manually - // through an additional ".flush()" property. - view.debounce = manualDebounce(); + if (overrideDebounce) { + // Replace the view to use a custom debounce function that can be triggered manually + // through an additional ".flush()" property. + view.debounce = manualDebounce(); + } return { toolbox, diff --git a/devtools/client/shared/inplace-editor.js b/devtools/client/shared/inplace-editor.js index d115238a8845..475e65c49b93 100644 --- a/devtools/client/shared/inplace-editor.js +++ b/devtools/client/shared/inplace-editor.js @@ -122,6 +122,7 @@ function isKeyIn(key, ...keys) { * This function is called before the editor has been torn down. * @param {Function} options.destroy: * Called when the editor is destroyed and has been torn down. + * This may be called with the return value of the options.done callback (if it is passed). * @param {Function} options.contextMenu: * Called when the user triggers a contextmenu event on the input. * @param {Object} options.advanceChars: @@ -476,8 +477,12 @@ class InplaceEditor extends EventEmitter { /** * Get rid of the editor. + * + * @param {*|null} doneCallResult: When #clear is called after calling #apply, this will + * be the returned value of the call to options.done that is done there. + * Will be null when options.done is undefined. */ - #clear() { + #clear(doneCallResult) { if (!this.input) { // Already cleared. return; @@ -499,7 +504,7 @@ class InplaceEditor extends EventEmitter { delete this.elt; if (this.destroy) { - this.destroy(); + this.destroy(doneCallResult); } } @@ -1157,8 +1162,8 @@ class InplaceEditor extends EventEmitter { ) { this.#acceptPopupSuggestion(); } else { - this.#apply(); - this.#clear(); + const onApplied = this.#apply(); + this.#clear(onApplied); } }; @@ -1363,7 +1368,7 @@ class InplaceEditor extends EventEmitter { } } - this.#apply(direction, key); + const onApplied = this.#apply(direction, key); // Close the popup if open if (this.popup && this.popup.isOpen) { @@ -1389,7 +1394,7 @@ class InplaceEditor extends EventEmitter { } } - this.#clear(); + this.#clear(onApplied); } else if (isKeyIn(key, "ESCAPE")) { // Cancel and blur ourselves. // Now we don't want to suggest anything as we are moving out. @@ -1399,8 +1404,8 @@ class InplaceEditor extends EventEmitter { this.#hideAutocompletePopup(); } else { this.cancelled = true; - this.#apply(); - this.#clear(); + const onApplied = this.#apply(); + this.#clear(onApplied); } prevent = true; event.stopPropagation();