From c1b72d4f7a465d858bfb4273e7cf2d3ff7d11221 Mon Sep 17 00:00:00 2001 From: Sebastian Zartner Date: Wed, 10 Feb 2021 20:45:44 +0000 Subject: [PATCH] Bug 1297633 - Fixed editing of SVG elements in Inspector. r=jdescottes Differential Revision: https://phabricator.services.mozilla.com/D102844 --- devtools/client/inspector/markup/markup.js | 8 +- .../client/inspector/markup/test/browser.ini | 1 + .../test/browser_markup_html_edit_04.js | 97 +++++++++++++++++++ devtools/server/actors/inspector/walker.js | 12 ++- 4 files changed, 110 insertions(+), 8 deletions(-) create mode 100644 devtools/client/inspector/markup/test/browser_markup_html_edit_04.js diff --git a/devtools/client/inspector/markup/markup.js b/devtools/client/inspector/markup/markup.js index c1d044823559..1e117d26f01b 100644 --- a/devtools/client/inspector/markup/markup.js +++ b/devtools/client/inspector/markup/markup.js @@ -1767,7 +1767,9 @@ MarkupView.prototype = { this.cancelReselectOnRemoved(); // Get the removedNode index in its parent node to reselect the right node. - const isHTMLTag = removedNode.tagName.toLowerCase() === "html"; + const isRootElement = ["html", "svg"].includes( + removedNode.tagName.toLowerCase() + ); const oldContainer = this.getContainer(removedNode); const parentContainer = this.getContainer(removedNode.parentNode()); const childIndex = parentContainer @@ -1781,7 +1783,7 @@ MarkupView.prototype = { mutation.removed && mutation.removed.some(n => n === removedNode); if ( mutation.type === "childList" && - (containsRemovedNode || isHTMLTag) + (containsRemovedNode || isRootElement) ) { isNodeRemovalMutation = true; break; @@ -1798,7 +1800,7 @@ MarkupView.prototype = { // selection. if ( this.inspector.selection.nodeFront === parentContainer.node || - (this.inspector.selection.nodeFront === removedNode && isHTMLTag) + (this.inspector.selection.nodeFront === removedNode && isRootElement) ) { const childContainers = parentContainer.getChildContainers(); if (childContainers?.[childIndex]) { diff --git a/devtools/client/inspector/markup/test/browser.ini b/devtools/client/inspector/markup/test/browser.ini index 955dc01cf4f7..b0a2241c4efd 100644 --- a/devtools/client/inspector/markup/test/browser.ini +++ b/devtools/client/inspector/markup/test/browser.ini @@ -166,6 +166,7 @@ skip-if = verify [browser_markup_html_edit_01.js] [browser_markup_html_edit_02.js] [browser_markup_html_edit_03.js] +[browser_markup_html_edit_04.js] [browser_markup_html_edit_undo-redo.js] [browser_markup_image_tooltip.js] [browser_markup_image_tooltip_mutations.js] diff --git a/devtools/client/inspector/markup/test/browser_markup_html_edit_04.js b/devtools/client/inspector/markup/test/browser_markup_html_edit_04.js new file mode 100644 index 000000000000..2329a09199c5 --- /dev/null +++ b/devtools/client/inspector/markup/test/browser_markup_html_edit_04.js @@ -0,0 +1,97 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// Test that outerHTML editing keybindings work as expected and that the +// root element can be edited correctly. + +const TEST_URL = + "data:image/svg+xml," + + '' + + '' + + ""; + +requestLongerTimeout(2); + +add_task(async function() { + const { inspector, testActor } = await openInspectorForURL(TEST_URL); + + inspector.markup._frame.focus(); + + info("Check that editing the element works like other nodes"); + await testDocumentElement(inspector, testActor); + + info("Check (again) that editing the element works like other nodes"); + await testDocumentElement2(inspector, testActor); +}); + +async function testDocumentElement(inspector, testActor) { + const currentDocElementOuterHTML = await testActor.eval( + "document.documentElement.outerHTML" + ); + const docElementSVG = + '' + + '' + + ""; + const docElementFront = await inspector.markup.walker.documentElement(); + + const onReselected = inspector.markup.once("reselectedonremoved"); + await inspector.markup.updateNodeOuterHTML( + docElementFront, + docElementSVG, + currentDocElementOuterHTML + ); + await onReselected; + + is( + await testActor.getAttribute("svg", "width"), + "200", + " width has been updated" + ); + is( + await testActor.getAttribute("svg", "height"), + "200", + " height has been updated" + ); + is( + await testActor.getProperty("svg", "outerHTML"), + docElementSVG, + " markup has been updated" + ); +} + +async function testDocumentElement2(inspector, testActor) { + const currentDocElementOuterHTML = await testActor.eval( + "document.documentElement.outerHTML" + ); + const docElementSVG = + '' + + '' + + ""; + const docElementFront = await inspector.markup.walker.documentElement(); + + const onReselected = inspector.markup.once("reselectedonremoved"); + inspector.markup.updateNodeOuterHTML( + docElementFront, + docElementSVG, + currentDocElementOuterHTML + ); + await onReselected; + + is( + await testActor.getAttribute("svg", "width"), + "300", + " width has been updated" + ); + is( + await testActor.getAttribute("svg", "height"), + "300", + " height has been updated" + ); + is( + await testActor.getProperty("svg", "outerHTML"), + docElementSVG, + " markup has been updated" + ); +} diff --git a/devtools/server/actors/inspector/walker.js b/devtools/server/actors/inspector/walker.js index 5240b501718a..d6f1bdb125b0 100644 --- a/devtools/server/actors/inspector/walker.js +++ b/devtools/server/actors/inspector/walker.js @@ -1708,7 +1708,9 @@ var WalkerActor = protocol.ActorClassWithSpec(walkerSpec, { // created nodes can be adopted into rawNode.parentNode. parser = new win.DOMParser(); } - const parsedDOM = parser.parseFromString(value, "text/html"); + + const mimeType = rawNode.tagName === "svg" ? "image/svg+xml" : "text/html"; + const parsedDOM = parser.parseFromString(value, mimeType); const parentNode = rawNode.parentNode; // Special case for head and body. Setting document.body.outerHTML @@ -1730,8 +1732,8 @@ var WalkerActor = protocol.ActorClassWithSpec(walkerSpec, { rawNode.outerHTML = value; } } else if (node.isDocumentElement()) { - // Unable to set outerHTML on the document element. Fall back by - // setting attributes manually, then replace the body and head elements. + // Unable to set outerHTML on the document element. Fall back by + // setting attributes manually. Then replace all the child nodes. const finalAttributeModifications = []; const attributeModifications = {}; for (const attribute of rawNode.attributes) { @@ -1747,8 +1749,8 @@ var WalkerActor = protocol.ActorClassWithSpec(walkerSpec, { }); } node.modifyAttributes(finalAttributeModifications); - rawNode.replaceChild(parsedDOM.head, rawNode.querySelector("head")); - rawNode.replaceChild(parsedDOM.body, rawNode.querySelector("body")); + + rawNode.replaceChildren(...parsedDOM.firstElementChild.childNodes); } else { // eslint-disable-next-line no-unsanitized/property rawNode.outerHTML = value;