If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

paste outer HTML should be undoable

NEW
Unassigned

Status

()

Firefox
Developer Tools: Inspector
P3
normal
3 years ago
2 years ago

People

(Reporter: Florent Fayolle, Unassigned)

Tracking

36 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [btpp-backlog])

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:34.0) Gecko/20100101 Firefox/34.0
Build ID: 20141110195804

Steps to reproduce:

1. Copy this code (without the quotes): "<p>foo</p>"
2. Open the Devtools inspector panel on this page
3. Select some elements (let's say #title > a)
4. Right-Click -> "Paste Outer HTML"
    |=> Bugzilla@Mozilla should be replaced with "Foo"
5. Press Ctrl-Z


Actual results:

The previous element should be restored.
(Reporter)

Updated

3 years ago
Component: Untriaged → Developer Tools: Inspector
OS: Linux → All
Hardware: x86_64 → All
Version: 34 Branch → 36 Branch
(Reporter)

Comment 1

3 years ago
Created attachment 8530681 [details] [diff] [review]
1100001.patch (WIP)

WIP. Almost done, still have some trouble with |node.nextSibling()| in the patch (I have to understand why it doesn't return the node we expect).

Florent
(Reporter)

Comment 2

3 years ago
Comment on attachment 8530681 [details] [diff] [review]
1100001.patch (WIP)

Review of attachment 8530681 [details] [diff] [review]:
-----------------------------------------------------------------

I took a little look at NodeFront#_next, it looks like _next and _prev are inverted in some (all?) situations. For example when I edit the <h2> tag of this page: http://perdu.com

It appears that the _next and _prev members are set through reparent() ( https://hg.mozilla.org/mozilla-central/file/2273193cc525/toolkit/devtools/server/actors/inspector.js#l885 ), but I don't understand the algorithm.

Patrick, do you think using |NodeFront#nextSibling()| in markup-view.js is the right way for this patch? Or should I request it through |this.walker.nextSibling(node)|?

Florent
Attachment #8530681 - Flags: feedback?(pbrosset)
Comment on attachment 8530681 [details] [diff] [review]
1100001.patch (WIP)

Review of attachment 8530681 [details] [diff] [review]:
-----------------------------------------------------------------

I hope my comment below answer your question about using _next and _prev. In short: you shouldn't.

::: toolkit/devtools/server/actors/inspector.js
@@ +752,5 @@
> +  /**
> +   * Returns the next sibling NodeFront for this NodeFront.
> +   */
> +  nextSibling: function() {
> +    return this._next;

this._next isn't guarantied to be the next sibling. In fact, for any NodeFront, the _prev and _next properties are only used internally to be able to reparent nodes, when processing mutations, or creating a new front.
In fact, they are just used to build a linked list of known nodes.

You can't assume that all children nodes of a node are known, and are in the right order, so you shouldn't expose these 2 properties publicly here.

Instead, use the walker actor's previousSibling and nextSibling methods if needed.
Attachment #8530681 - Flags: feedback?(pbrosset)
(Reporter)

Comment 4

3 years ago
Thanks, I will update the code soon.

Florent
There is also Bug 998933 opened for undoing 'edit as html' which is pretty much the same thing, and has some related discussion
See Also: → bug 998933

Comment 6

2 years ago
Mozilla/5.0 (Windows NT 10.0; rv:44.0) Gecko/20100101 Firefox/44.0

Steps to reproduce:

1. Copy this code (without the quotes): "<p>foo</p>"
2. Open the Devtools inspector panel on this page
3. Select some elements (let's say #title > a)
4. Right-Click -> "Paste Outer HTML"
    |=> Bugzilla@Mozilla should be replaced with "Foo"
5. Press Ctrl-Z


Actual results:

The previous element should be restored.
Filter on CLIMBING SHOES
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Filter on CLIMBING SHOES
Whiteboard: [btpp-backlog]
You need to log in before you can comment on or make changes to this bug.