Bug 1670907 - moving buttons in customize mode should not disable them, r=mconley

When we move items in customize mode, we unwrap them, then move them, then wrap
them again. The unwrapping sets the `command` attribute back on the button, and
that also sets the `disabled` attribute on the button. The wrapping removes the
`command` attribute but leaves the `disabled` attribute.
When the `disabled` attribute is removed from the command, this does not
propagate to the button because the command attribute has not been put back.

To fix this, the patch avoids adding and immediately removing the command
attribute when moving items while in customize mode.

Differential Revision: https://phabricator.services.mozilla.com/D93338
This commit is contained in:
Gijs Kruitbosch 2020-10-14 14:04:02 +00:00
parent dffea8097c
commit b85bab8fe4
2 changed files with 27 additions and 7 deletions

View File

@ -897,7 +897,7 @@ CustomizeMode.prototype = {
// keep strong refs to it in CustomizableUI (can't iterate of
// WeakMaps), and there's the question of what behavior
// wrappers should have if consumers keep hold of them.
let unwrappedPaletteItem = this.unwrapToolbarItem(paletteChild);
let unwrappedPaletteItem = this.unwrapToolbarItem(paletteChild, true);
this._stowedPalette.appendChild(unwrappedPaletteItem);
}
@ -1067,12 +1067,12 @@ CustomizeMode.prototype = {
return wrapper;
},
deferredUnwrapToolbarItem(aWrapper) {
deferredUnwrapToolbarItem(aWrapper, aReconnectCommands) {
return new Promise(resolve => {
dispatchFunction(() => {
let item = null;
try {
item = this.unwrapToolbarItem(aWrapper);
item = this.unwrapToolbarItem(aWrapper, aReconnectCommands);
} catch (ex) {
Cu.reportError(ex);
}
@ -1081,7 +1081,7 @@ CustomizeMode.prototype = {
});
},
unwrapToolbarItem(aWrapper) {
unwrapToolbarItem(aWrapper, aReconnectCommands) {
if (aWrapper.nodeName != "toolbarpaletteitem") {
return aWrapper;
}
@ -1110,7 +1110,7 @@ CustomizeMode.prototype = {
toolbarItem.checked = true;
}
if (aWrapper.hasAttribute("itemcommand")) {
if (aWrapper.hasAttribute("itemcommand") && aReconnectCommands) {
let commandID = aWrapper.getAttribute("itemcommand");
toolbarItem.setAttribute("command", commandID);
@ -1222,7 +1222,7 @@ CustomizeMode.prototype = {
_unwrapItemsInArea(target) {
for (let toolbarItem of target.children) {
if (this.isWrappedToolbarItem(toolbarItem)) {
this.unwrapToolbarItem(toolbarItem);
this.unwrapToolbarItem(toolbarItem, true);
}
}
},
@ -1232,7 +1232,7 @@ CustomizeMode.prototype = {
for (let target of this.areas) {
for (let toolbarItem of target.children) {
if (this.isWrappedToolbarItem(toolbarItem)) {
await this.deferredUnwrapToolbarItem(toolbarItem);
await this.deferredUnwrapToolbarItem(toolbarItem, true);
}
}
this._removeDragHandlers(target);

View File

@ -52,3 +52,23 @@ add_task(async function test_disable_commands() {
);
}
});
/**
* When buttons are connected to a command, they should not get
* disabled just because we move them.
*/
add_task(async function test_dont_disable_when_moving() {
await startCustomizing();
CustomizableUI.addWidgetToArea("print-button", "nav-bar");
ok(
!document.getElementById("print-button").hasAttribute("disabled"),
"Should not have disabled attribute when moving via API in customize mode"
);
await gCustomizeMode.reset();
ok(
!document.getElementById("print-button").hasAttribute("disabled"),
"Should not have disabled attribute when resetting in customize mode"
);
await endCustomizing();
});