2064213 - Inspector: F2 invokes “Edit as HTML” on unsupported nodes and displays stale content from previous edit

Status: RESOLVED FIXED

(DevTools :: Inspector: Rules, defect, P3)

Description

[Peter Barney]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:153.0) Gecko/20100101 Firefox/153.0

Steps to reproduce:

1. In the Inspector, select a normal HTML element, such as a <span> or <pre>.
2. Press F2 to invoke "Edit as HTML."
3. Modify the content, or leave it unchanged.
4. Press F2 to commit the edit, or Esc to cancel it.
5. Select a node for which "Edit as HTML" is unavailable, such as:

• a text node,
• a [whitespace] node, or
• a CSS rule shown inside a <style> element.

6. Press F2.

Actual results:

The editor opens and displays the content from the element edited in steps 1–4, including any changes made there, even if the edit was canceled.

Pressing F2 again to save has no effect.

If F2 is pressed again, the same stale content is still shown in the editor.

The context menu equivalent works as expected on these nodes: "Edit as HTML" changes to "Edit as XML" and is disabled.

Expected results:

The F2 shortcut should follow the same eligibility rules as the context menu command.

If the currently selected node can't be edited using "Edit as HTML," pressing F2 should do nothing, or otherwise indicate that the operation is unavailable.

Why this is a problem:

It can be misleading when working with <style> elements.

For example, after editing a valid <style> element and checking the result in the browser, it is easy to return to the Inspector, select one of the CSS-rule entries displayed inside that element, and press F2.

The editor seems to accept the changes, and those changes remain visible if the editor is reopened, but they're never applied to the page. This makes it look like the CSS edit succeeded when it didn't.

Comment 1

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'DevTools::Inspector: Rules' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Here's where we handle the F2 key press https://searchfox.org/firefox-main/rev/6fd3453f587cfcd74637bad0b7b67f3d4680e679/devtools/client/inspector/markup/markup.js#122-124

"markupView.edit.key": markupView => {
  markupView.beginEditingHTML(markupView._selectedContainer.node);
},

The context menu is disabled in those cases https://searchfox.org/firefox-main/rev/6fd3453f587cfcd74637bad0b7b67f3d4680e679/devtools/client/inspector/markup/markup-context-menu.js#748-751,763

const isFragment = this.selection.isDocumentFragmentNode();
const isAnonymous = this.selection.isNativeAnonymousNode();
const isElement =
  this.selection.isElementNode() && !this.selection.isPseudoElementNode();
...
    disabled: isAnonymous || (!isElement && !isFragment),

we should share these conditions and re-use them for the F2 case

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 2064213 - [devtools] Make Edit HTML via F2 key press check if node HTML can be edited. r=#devtools

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ceba34a17023
https://hg.mozilla.org/integration/autoland/rev/e55094580b8a
[devtools] Make Edit HTML via F2 key press check if node HTML can be edited. r=devtools-reviewers,bomsy

Comment 5

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/e55094580b8a