112922 - Ability to edit and split #text nodes

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, enhancement)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

It would be great if #text nodes could be edited with the DOM inspector.

Comment 1

[Alex Vincent [:WeirdAl]]

I'm willing to work on this one.

Comment 2

[Alex Vincent [:WeirdAl]]

Sorry, forgot to cc myself and explain the subject change.

Splitting text nodes is also important, because that lets us insert an element 
inside the text node.  Ideally, I'd like to make the domNode.xul bxText have a 
non-readonly equivalent, specifically for text, comment and CDATA nodes.  I 
tried this, however, and ran into a glitch.  Specifically, I couldn't actually 
set a context attribute for the textbox element in question and have it 
override the default context menu in a 1.1a+ build.  Maybe they've changed that 
since, but I'm not sure of that.

I'm going to need help from XBL to do this.  (1) I need to override the default 
context menu for the textbox.  (2) I need to bind the textbox's contents to the 
corresponding node's value -- so that when one changes, even with a single 
keystroke, the other reflects that change instantly.

Is anyone willing to write XBL code to help me do these two things?  If someone 
can do that, I can do the rest in XUL/JS and make this work.

Comment 3

[Alex Vincent [:WeirdAl]]

Heh, I also need someone to show me how to reach the selected text node.

Comment 4

[Christopher Aillon (sabbatical, not receiving bugmail)]

You should not need xbl to do this.

Comment 5

[Alex Vincent [:WeirdAl]]

Agreed, after some tinkering.  I've got a working text edit/split feature for 
DOM Inspector, without XBL.

There's only two problems.  (1) the selectionStart property is broken for XUL 
multiline textboxes (bug 58850).  And (2), the new feature breaks undo/redo in 
the process...

For obvious reasons I cannot deliver a patch based on that.  Undo/redo is a 
vital feature.  I'm about to file a bug regarding undo/redo in DOM Inspector vs 
Composer (as there's an RFE to tie the two together), and I'll bring up some of 
these issues there.

Comment 6

[Alex Vincent [:WeirdAl]]

I've been examining this issue again for undo/redo, and there's a couple 
fundamental issues with multiple textboxes which need to be addressed.  
Otherwise, I just don't see how this is really doable.

* It would be really nice if each textbox or textbox.inputField exposed an 
nsITransactionManager by a binding, and executed its transactions through 
nsITransaction objects available in the nsITransactionManager.

* There are three different ways to undo a <textbox /> transaction.  The first 
is by context menu, the Undo menu item.  The second is by key command, 
accelkey+Z.  The third is by application command, such as Edit, Undo.  Ideally, 
all three cmd_undo (and corresponding cmd_redo) procedures would fire an event 
through (but not necessarily targeted at) the textbox.

Comment 7

[kinmoz]

FYI, I checked in a fix for bug 88049 back during the 1.3 timeframe, so
selectionStart/End should now work for textareas (multi-line-textfields).

If you want to be able to insert text anywhere in the text widget and have it
undoable, I think we'll need to expose the insertion methods mentioned in bug
178683.

Comment 8

[Alex Vincent [:WeirdAl]]

If editing/splitting text nodes were to happen in a modal dialog box (say, we
added a menu option to the bottom of the normal textbox for activating an edit
mode), then I wouldn't need to tie the textbox's txmgr to the master application.

A modal dialog may be the only efficient way to get this job done.  The user
might not like having to go through the extra steps, and localizing dialogs
isn't something I haven't figured out yet, but it means I wouldn't depend on
179621, 180840, or 204793.  I could also work around bug 180512 (under protest...)

Comment 9

[Boris Zbarsky [:bzbarsky]]

*** Bug 245226 has been marked as a duplicate of this bug. ***

Comment 10

[Norbert]

(I'm also interested in these improvements.)

Comment 11

[Alex Vincent [:WeirdAl]]

If anyone wants to take over this bug and generate a patch for it, I can write 
out what my line of thinking was on how to implement this.

Comment 12

[Jason Barnabe (np)]

Attachment: edit text nodes v1

This patch makes the text node view editable. The changes will be reflected onchange (which happens once the textbox loses focus after a change). Undo/Redo from the Edit menu affects each onchange, Undo/Redo from the textbox context menu is just regular textbox Undo/Redo.

Since comment, cdata, text, and PI nodes are the only ones that have nodeValues that are ever not null (attributes can too, but they're not displayed in the tree), I've updated it to show the big text box for those node types and removed the "Node Value" box from all the others.

I'd like to handle splitting text nodes separately.

Comment 13

[timeless]

Comment on attachment 223616 [details] [diff] [review]
edit text nodes v1

seems fine to me.

Comment 14

[[e-mail]]

(In reply to comment #12)
>Undo/Redo from the Edit menu affects each onchange
Aside: I wonder whether it would be worth teaching Inspector about controllers?

Comment 15

[[e-mail]]

Comment on attachment 223616 [details] [diff] [review]
edit text nodes v1

>   set subject(aObject) 
>   {
>+    // the node value's textbox won't fire onchange when we change subjects, so 
>+    // let's fire it. this won't do anything if it wasn't actually changed
>+    viewer.pane.panelset.execCommand('cmdEditNodeValue');
But note that if you switch to JavaScript Object then your changes are lost.

>+        var txb = document.getElementById("txbTextNodeValue");
>+        txb.value = aObject.nodeValue;
Nit: I know all you did was reindent this, but In your new code you don't bother assigning this element to a local, so you might as well match it here.

>+      document.getElementById("txbTextNodeValue").value = this.subject.nodeValue;
Wrap this line perhaps?

Comment 16

[[e-mail]]

(In reply to comment #15)
>But note that if you switch to JavaScript Object then your changes are lost.
Looks like this is a one-liner in the destroy method.

Comment 17

[Jason Barnabe (np)]

Attachment: patch v2

Updated to comments.

Comment 18

[:Gavin Sharp [email: [e-mail]]]

Checked in on the trunk.