Closed Bug 112922 Opened 20 years ago Closed 16 years ago

Ability to edit and split #text nodes

Categories

(Other Applications :: DOM Inspector, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9alpha

People

(Reporter: Biesinger, Assigned: jason.barnabe)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file, 1 obsolete file)

It would be great if #text nodes could be edited with the DOM inspector.
Status: NEW → ASSIGNED
Target Milestone: --- → Future
I'm willing to work on this one.
Blocks: 109682
Summary: Ability to edit #text nodes → Ability to edit and split #text nodes
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.
OS: Windows 98 → All
Heh, I also need someone to show me how to reach the selected text node.
You should not need xbl to do this.
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.
Depends on: 58850
Depends on: 180512
Depends on: 180840
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.
Assignee: hewitt → ajvincent
Status: ASSIGNED → NEW
Keywords: helpwanted
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.
Depends on: 179621
Depends on: 204793
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...)
No longer depends on: 179621, 180512, 180840, 204793
*** Bug 245226 has been marked as a duplicate of this bug. ***
(I'm also interested in these improvements.)
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.
Product: Core → Other Applications
Attached patch edit text nodes v1 (obsolete) — Splinter Review
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.
Assignee: ajvincent → jason_barnabe
Status: NEW → ASSIGNED
Attachment #223616 - Flags: superreview?(neil)
Attachment #223616 - Flags: review?(timeless)
Keywords: helpwanted
Comment on attachment 223616 [details] [diff] [review]
edit text nodes v1

seems fine to me.
Attachment #223616 - Flags: review?(timeless) → review+
(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 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?
Attachment #223616 - Flags: superreview?(neil) → superreview+
(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.
Attached patch patch v2Splinter Review
Updated to comments.
Attachment #223616 - Attachment is obsolete: true
Whiteboard: [checkin needed]
Target Milestone: Future → mozilla1.9alpha
Checked in on the trunk.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Hardware: PC → All
Resolution: --- → FIXED
Whiteboard: [checkin needed]
QA Contact: timeless → dom-inspector
You need to log in before you can comment on or make changes to this bug.