fix: duplicate buttons on desk (#16941)

cypress/integration/custom_buttons.js

@@ -0,0 +1,59 @@
+const test_button_names = [
+	"Metallica",
+	"Pink Floyd",
+	"Porcupine Tree (the GOAT)",
+	"AC / DC",
+	`Electronic Dance "music"`,
+];
+
+const add_button = (label, group = "TestGroup") => {
+	cy.window()
+		.its("cur_frm")
+		.then((frm) => {
+			frm.add_custom_button(label, () => {}, group);
+		});
+};
+
+const check_button_count = (label, group = "TestGroup") => {
+	// Verify main buttons
+	cy.findByRole("button", { name: group }).click();
+	cy.get(`[data-label="${encodeURIComponent(label)}"]`)
+		.should("have.length", 1)
+		.should("be.visible");
+
+	// Verify dropdown buttons in mobile view
+	cy.viewport(420, 900);
+	const dropdown_btn_label = `${group} > ${label}`;
+	cy.get(".menu-btn-group > .btn").click();
+	cy.get(`[data-label="${encodeURIComponent(dropdown_btn_label)}"]`)
+		.should("have.length", 1)
+		.should("be.visible");
+
+	//reset viewport
+	cy.viewport(
+		Cypress.config("viewportWidth"),
+		Cypress.config("viewportHeight")
+	);
+};
+
+describe(
+	"Custom group button behaviour on desk",
+	{ scrollBehavior: false }, // speeds up the test
+	() => {
+		before(() => {
+			cy.login();
+			cy.visit(`/app/note/new`);
+		});
+
+		test_button_names.forEach((button_name) => {
+			it(`Custom button works with name '${button_name}'`, () => {
+				add_button(button_name);
+				check_button_count(button_name);
+
+				// duplicate button shouldn't be added
+				add_button(button_name);
+				check_button_count(button_name);
+			});
+		});
+	}
+);

cypress/integration/folder_navigation.js

@@ -1,3 +1,13 @@
+const click_menu_button = (name) => {
+	cy.get('.menu-btn-group > .btn').click();
+	cy.get(`.menu-btn-group [data-label="${encodeURIComponent(name)}"]`).click();
+}
+
+const click_action_button = (name) => {
+	cy.findByRole('button', {name: 'Actions'}).click();
+	cy.get(`.actions-btn-group [data-label="${encodeURIComponent(name)}"]`).click();
+}
+
 context('Folder Navigation', () => {
 	before(() => {
 		cy.visit('/login');
@@ -15,10 +25,9 @@ context('Folder Navigation', () => {
 		cy.get('.filter-action-buttons > div > .btn-primary').findByText('Apply Filters').click();
 
 		//Adding folder (Test Folder)
-		cy.get('.menu-btn-group > .btn').click();
-		cy.get('.menu-btn-group [data-label="New Folder"]').click();
+		click_menu_button("New Folder");
 		cy.fill_field('value', 'Test Folder');
-		cy.click_modal_primary_button('Create'); 
+		cy.click_modal_primary_button('Create');
 	});
 
 	it('Navigating the nested folders, checking if the URL formed is correct, checking if the added content in the child folder is correct', () => {
@@ -30,10 +39,9 @@ context('Folder Navigation', () => {
 		cy.visit('/app/file/view/home/Attachments');
 
 		//Adding folder inside the attachments folder
-		cy.get('.menu-btn-group > .btn').click();
-		cy.get('.menu-btn-group [data-label="New Folder"]').click();
+		click_menu_button("New Folder");
 		cy.fill_field('value', 'Test Folder');
-		cy.click_modal_primary_button('Create'); 
+		cy.click_modal_primary_button('Create');
 
 		//Navigating inside the added folder in the Attachments folder
 		cy.get('[title="Test Folder"] > span').click();
@@ -59,16 +67,14 @@ context('Folder Navigation', () => {
 		}).as('file_deleted');
 
 		//Deleting the added file from the Test folder
-		cy.findByRole('button', {name: 'Actions'}).click();
-		cy.get('.actions-btn-group [data-label="Delete"]').click();
+		click_action_button("Delete")
 		cy.click_modal_primary_button('Yes');
 		cy.wait('@file_deleted');
 
 		//Deleting the Test Folder
 		cy.visit('/app/file/view/home/Attachments');
 		cy.get('.list-row-checkbox').eq(0).click();
-		cy.findByRole('button', {name: 'Actions'}).click();
-		cy.get('.actions-btn-group [data-label="Delete"]').click();  
+		click_action_button("Delete")
 		cy.click_modal_primary_button('Yes');
 		cy.wait('@file_deleted');
 	});
@@ -77,8 +83,7 @@ context('Folder Navigation', () => {
 	//Deleting the Test Folder added in the home directory
 		cy.visit('/app/file/view/home');
 		cy.get('.level-left > .list-subject > .file-select >.list-row-checkbox').eq(0).click({force: true, delay: 500});
-		cy.findByRole('button', {name: 'Actions'}).click();
-		cy.get('.actions-btn-group [data-label="Delete"]').click();
+		click_action_button("Delete")
 		cy.click_modal_primary_button('Yes');
 	});
-}); 
+});

frappe/public/js/frappe/ui/alt_keyboard_shortcuts.js

@@ -128,7 +128,7 @@ frappe.ui.keys.AltShortcutGroup = class AltShortcutGroup {
 			return !this.is_taken(letter) && is_valid_char;
 		});
 		if (!shortcut_letter) {
-			$text_el.attr('data-label', text_content);
+			$text_el.attr('data-label', encodeURIComponent(text_content));
 			return;
 		}
 		for (let key in this.shortcuts_dict) {
@@ -152,7 +152,7 @@ frappe.ui.keys.AltShortcutGroup = class AltShortcutGroup {
 	}
 
 	underline_text(shortcut) {
-		shortcut.$text_el.attr('data-label', shortcut.text);
+		shortcut.$text_el.attr('data-label', encodeURIComponent(shortcut.text));
 		let underline_el_found = false;
 		let text_html = shortcut.text.split('').map(letter => {
 			if (letter === shortcut.letter && !underline_el_found) {

frappe/public/js/frappe/ui/page.js

@@ -414,7 +414,7 @@ frappe.ui.Page = class Page {
 			parent.parent().removeClass("hide");
 		}
 
-		let $link = this.is_in_group_button_dropdown(parent, 'li > a.grey-link', label);
+		let $link = this.is_in_group_button_dropdown(parent, 'li > a.grey-link > span', label);
 		if ($link) return $link;
 
 		let $li;
@@ -510,12 +510,10 @@ frappe.ui.Page = class Page {
 
 		if (!label || !parent) return false;
 
-		const result = $(parent).find(`${selector}:contains('${label}')`)
-			.filter(function() {
-				let item = $(this).html();
-				return $(item).attr('data-label') === label;
-			});
-		return result.length > 0 && result;
+		const item_selector = `${selector}[data-label='${encodeURIComponent(label)}']`;
+
+		const existing_items = $(parent).find(item_selector);
+		return existing_items?.length > 0;
 	}
 
 	clear_btn_group(parent) {