Open
Bug 1226021
Opened 9 years ago
Updated 2 years ago
[markup-view] Undo for tag change
Categories
(DevTools :: Inspector, enhancement, P2)
Tracking
(Not tracked)
NEW
People
(Reporter: grisha, Unassigned)
Details
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20151110004043
Steps to reproduce:
1. Rename tag
2. Try to undo changes
Reference to client/markupview/markup-view.js:
https://dxr.mozilla.org/mozilla-central/source/devtools/client/markupview/markup-view.js#2954
Actual results:
Undo isn't happening.
Expected results:
Tag should be backed to its initial value.
Reporter | ||
Updated•9 years ago
|
Component: Untriaged → Developer Tools: Inspector
Reporter | ||
Comment 1•9 years ago
|
||
From bug 1221183 comment 21:
> If we change attribute, rename tag and then undo - there is an error as was before with attributes
> and tag change at the same time and then undo. Somehow we should change node after creating
> actions in doMods / undoMods.
Comment 2•9 years ago
|
||
(In reply to Grisha Pushkov from comment #1)
> From bug 1221183 comment 21:
> > If we change attribute, rename tag and then undo - there is an error as was before with attributes
> > and tag change at the same time and then undo. Somehow we should change node after creating
> > actions in doMods / undoMods.
This is getting complicated. Basically, anytime we want to undo something on a node that was deleted/re-created, this will happen. If you changed a node's attributes, and then edited it as HTML, or deleted it, or changed its tagname, we would have to somehow track some reference to this node so that if you went back in history, we'd be able to know which node needs to have its attributes changed.
For instance:
1 - change node's attributes
2 - delete node
3 - undo (which would re-create the node by actually appending a new one)
4 - undo (which should revert the attribute changes, but on the new node, the original one was deleted).
Or whenever a node is deleted/changed in the inspector, we make sure to keep a reference to it so it doesn't get garbage collected, so we can actually re-append the same one later on undo.
What that means however, is that we would need to move the whole undo/redo logic to the server (in devtools/server/actors/inspector.js).
I think it makes more sense for it to live there anyway, but that's a much larger project.
In the meantime, I think we could still implement tag edit undo client-side, by reusing the same doMods/undoMods system we have today. But whenever a node gets its tag changed, we should perhaps reset the undo stack for this node, so that there's no risk of trying to undo an attribute change on a deleted node.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter | ||
Comment 3•9 years ago
|
||
> But whenever a node gets its tag changed, we should perhaps reset the undo stack for this node, so that
> there's no risk of trying to undo an attribute change on a deleted node.
And now I get the reason for existence of those try / catches, that I removed in bug 994555 (bug 994555 comment 30).
Comment 4•9 years ago
|
||
Triaging (filter on CLIMBING SHOES).
Priority: -- → P2
Whiteboard: [btpp-fix-later]
Updated•8 years ago
|
Blocks: top-inspector-bugs
Blocks: 1312444
Updated•6 years ago
|
Product: Firefox → DevTools
Updated•5 years ago
|
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•