Closed Bug 1220696 Opened 6 years ago Closed 3 months ago

textarea execcommand insertText not working properly

Categories

(Core :: DOM: Editor, defect, P3)

50 Branch
All
Other
defect

Tracking

()

RESOLVED FIXED
89 Branch
Tracking Status
firefox89 --- fixed

People

(Reporter: daniel.didusch, Assigned: masayuki)

References

(Blocks 1 open bug, )

Details

(Keywords: dev-doc-complete, parity-chrome, parity-edge, Whiteboard: [specification][type:bug], [wptsync upstream])

Attachments

(8 files)

WIP
2.65 KB, patch
Details | Diff | Splinter Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
What did you do?
================
<!doctype html>
<html>
	<head>
		<title>Test</title>
	</head>
	<body>
		<textarea designMode="on" id="textarea"></textarea>
		<a href="#" onclick="document.getElementById('textarea').focus(); document.execCommand('insertText', false, 'Text'); return false;">Insert text</a>
		<a href="#" onclick="document.getElementById('textarea').value = ''; return false;">Reset</a>
	</body>
</html>

What happened?
==============
Insert text works properly as long you don't change it's value

What should have happened?
==========================
Should also work after editing text

Is there anything else we should know?
======================================
Sounds like you're trying to report a bug in Firefox.
Product: Mozilla Developer Network → Firefox
You're right Luke Crouch, I missed that one, sorry for any inconvenience!
Blocks: 748307
Component: General → Editor
Product: Firefox → Core
Summary: textarea execcommand not working properly → textarea execcommand insertText not working properly
Here is a jsFiddle that I created that illustrates the bug, https://jsfiddle.net/dvo5pnvy/3/.

In chrome, safari, and edge, when pasting into the input text box, the string "This should be pasted." is pasted in, no matter what is in your clipboard. In firefox, however, whatever is in your clipboard is pasted in, even though the event fires.
Component: Editor → Untriaged
Product: Core → Firefox
Version: unspecified → 50 Branch
Firefox: 50.0a1, Build ID: 20160626030213
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:50.0) Gecko/20100101 Firefox/50.0

I have tested this issue on the latest Firefox (47.0) release and latest Nightly (50.0a1) build. When testing this I have used both provided test cases.
In the test case from comment 0, when pressing the "Insert Text" link, nothing happens. In Chrome when pressing the "Insert Text" link in the text box the "Text" string is displayed.
In the test case from comment 3, any copied string is pasted in the text box, but the "This should be pasted." string should be pasted.
Status: UNCONFIRMED → NEW
Component: Untriaged → Editor
Ever confirmed: true
Product: Firefox → Core
Flags: needinfo?(masayuki)
I'm not sure if comment 3's testcase is exactly same bug as this.

Here is the simplest testcase: https://jsfiddle.net/d_toybox/prkt4yhd/1/

Anyway, according to MDN, document.execCommand() is designed only for designMode="on" or "contenteditable". So, it's not working with <input> nor <textarea> must be intentional behavior in Gecko.
https://developer.mozilla.org/en-US/docs/Web/API/Document/execCommand

However, indeed, it makes incompatibility between other browsers. This is too bad. I guess that it's not so difficult to implement this, but I don't have much time because there are a lot of bugs in my queue...
Flags: needinfo?(masayuki)
According to the debugger, we hit here because there is no HTML editor in the testcase:
https://dxr.mozilla.org/mozilla-central/rev/6608e5864780589b25d5421c3d3673ab30c4c318/dom/html/nsHTMLDocument.cpp#3146-3149

And I guess we need to hack here:
https://dxr.mozilla.org/mozilla-central/rev/6608e5864780589b25d5421c3d3673ab30c4c318/dom/html/nsHTMLDocument.cpp#3193-3199

We need to use command manager for <input> or <textarea> instead of HTML editor's.

I guess that this bug is useful for Gecko Hands-on which will be held in Japan late this month. I'll research the detail more and new hacker should fix this bug if it's possible.
Assignee: nobody → masayuki
Thank you for looking into this!  Do you know what, if anything, the spec says here?
(In reply to Boris Zbarsky [:bz] from comment #7)
> Thank you for looking into this!  Do you know what, if anything, the spec
> says here?

Although, not yet reading whole of the the Editing API draft, I don't find about the target of execCommand. I feel odd to use "document."execCommand for an editable element, though. (<input>.execCommand makes sense for me...)
Duplicate of this bug: 241182
This bug becomes more important since setting `.value` now nukes undo history for <textarea>s and <input>s.

If developers want to preserve undo history for <textarea>s and <input>s, they would use `execcommand` on Webkit and directly use .value on Firefox.  Now, there is no way for preserving undo history while programmatically changing <textarea> or <input> values.
Please also refer to https://twitter.com/FakeU/status/901671863851429889 for illustration.
Whiteboard: [specification][type:bug] → [specification][type:bug][parity-blink][parity-edge]
Here is a simple editor with two buttons inserting text to either a `textarea` or a `contenteditable` element:
http://jsbin.com/baxusiw/edit?html,js,console,output

Same thing exported to Gist:
https://gist.github.com/Eccenux/4adb35f3a319c1df605690b2d6020e7a

Inserting Bold/Italic to textarea:
* FF (60): Doesn't work.
* Chrome (66): Works. Undo works.
* Opera (52): [same as Chrome].
* Edge (41): Works. Undo works.
* Safari: ? (untested, but other sources suggest it works)

Inserting Bold/Italic to contenteditable:
* FF (60): Works. Undo works.
* Chrome (66): Works. Undo works, but all changes are cumulated (all changes are reverted with one CTRL+Z).
* Opera (52): [same as Chrome].
* Edge (41): Works. Undo works.
* Safari: ?

So it seem like FF is left alone with IE, but in IE you can insert text into range and undo will work.
Thank you for comparing with the other browsers. I still think that "document." is odd. However, we should take same behavior.
I checked current design roughly, but I have no idea how to implement.

Currently, HTML edit commands registers with HTMLEditor instance as their context. If we take same approach, we need to register every supported command *per* <input> or <textarea> element. So, this wastes memory a lot and makes page load speed slower especially when there are a lot of <input> and <textarea> elements.

Perhaps, when active element of nsHTMLDocument has TextEditor, we should redirect the command to new command handler in editor? I don't want to separate the command handling path between HTMLEditor and TextEditor...
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #14)
> I checked current design roughly, but I have no idea how to implement.
> 
> Currently, HTML edit commands registers with HTMLEditor instance as their
> context. If we take same approach, we need to register every supported
> command *per* <input> or <textarea> element. So, this wastes memory a lot
> and makes page load speed slower especially when there are a lot of <input>
> and <textarea> elements.
> 
> Perhaps, when active element of nsHTMLDocument has TextEditor, we should
> redirect the command to new command handler in editor? I don't want to
> separate the command handling path between HTMLEditor and TextEditor...

If not contenteditable, this line always return false due to IsEditingOnAfterFlush

https://searchfox.org/mozilla-central/rev/11a2ae294f50049e12515b5821f5a396d951aacb/dom/html/nsHTMLDocument.cpp#2859

nsHTMLEdocument::execCommand looks for nsICommandManager of editor. And TextEditor has/registers EditorController and EditorCommands (*Element::GetController can return editor's controller).  But nsHTMLEditor::execCommand cannot find InsertPlaintextCommand::DoCommand in EditorCommands.

Although it isn't clear, since cmd_insertText doesn't register XUL's keyboard handler by XUL file, nsHTMLEdocument::execCommand cannot find/call EditorCommands's InsertPlaintextCommand.  (cmd_copy, cmd_paste and etc has XUL's keyboard handler).  But since it isn't clear, we need investigate more.
Since we don't have a list what command for execCommand we should allowed in textarea/input text.  At least, we might have to allow undo, redo and insertText on textarea/input
(In reply to Makoto Kato [:m_kato] from comment #16)
> Since we don't have a list what command for execCommand we should allowed in
> textarea/input text.  At least, we might have to allow undo, redo and
> insertText on textarea/input

Yeah, and perhaps, insertParagraphSeparator, insertLineBreak (only <textarea>), delete, forwarddelete and selectAll.
Attached patch WIPSplinter Review
Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)
Whiteboard: [specification][type:bug][parity-blink][parity-edge] → [specification][type:bug]
This caused an issue with pasting text into comments on Youtube in bug #1466391. 

Google also provided a test case: https://codepen.io/cheng-lee/pen/rKzbPx
See Also: → 1466391
Blocks: 1443902
I noticed on FF 65.0a1 the issue is still present and fails to insert text into textarea

const insert = document.execCommand('insertText', false, text);
console.log(insert); // false

Are there any updates?
(In reply to erosman from comment #21)
> I noticed on FF 65.0a1 the issue is still present and fails to insert text
> into textarea
> 
> const insert = document.execCommand('insertText', false, text);
> console.log(insert); // false
> 
> Are there any updates?

Just curious, although it's not standardized, why do you think that this bug is important? If the reason is serious and technical one, we should implement this as soon as possible.
I can't speak for erosman but I'm currently working on an extension that provides keyboard shortcuts to manipulate text in textarea, input and contenteditable elements. Changing the value of textareas using `elem.value = newvalue` seems to destroy history but using document.execCommand("insertText", ...) doesn't. Thus I'd find it nice if I was able to use document.execCommand on textareas.

I wouldn't say that this is very important to me though :).
> Just curious, although it's not standardized, why do you think that this bug is important? If the reason is serious and technical one, we should implement this as soon as possible.

It is not imperative as I worked around it but MDN does not mention that it is not supported.
https://developer.mozilla.org/en-US/docs/Web/API/Document/execCommand

The paste into textarea from an extension without above, involves getting textarea, cursor position and break textarea value, add new text and put it back together. It is a long way round, which would have been made easier with above API.
> seems to destroy history

Amusingly enough, it used to not do that in Firefox, but none of the other browsers were willing to support undo across .value sets, so we disabled it too...
>but none of the other browsers were willing to support undo across .value sets

That's interesting, has there been discussion about it or was it more like "we waited for others to do the same thing and they didn't"? If there were discussions, do you know if/where I could read them? I'd like to understand their rationale, Firefox clearly used to do the right thing IMO.
> I'd like to understand their rationale

It slowed down benchmarks like Speedometer that set .value repeatedly and measure the performance of the setter.

I'm not sure offhand whether the discussions were public.
To preserve textarea history, for Chrome I used to do execCommand and for Firefox just set .value, but now I am left with no other solution than changing the textarea to a contenteditable (with a lot of side effects) for preserving the undo stack in Firefox.

If execcommand on input elements works without needing a contenteditable, that would make programmatic manipulation of values in input/textarea much easier.
Thanks for the answers! I understand, not breaking undo stack is important for UX of Firefox users if execCommand() is used only for the other browsers since we've stopped appending history of .value setting as bzbarsky said.
I also came across another consideration. The work around I mentioned in Comment 24 works only for textarea & text input. It requires more code for contenteditable elements.
However, document.execCommand('insertText', false, text) works for all of them.
Sorry.. never mind Comment 30 document.execCommand('insertText', false, text) works for contenteditable elements
@erosman Could you share a fiddle with the workaround you talk about? I don't understand what do you mean by: "break textarea value, add new text and put it back together". Are you saying that it is possible to insert some text in textarea in Firefox and undo this (with CTLR+Z)?
@Maciej Jaros I meant break the textarea value into segments e.g. node.value.substring(), node.selectionStart & node.selectionEnd add the text in between and reset the textarea.value to the new text. This is nothing new and works for my intended purpose but it does not work for your purpose since it can not be undone. I haven't found a way for Firefox to keep the undo history.
I would have also preferred the simpler document.execCommand('insertText',..) that is supported by all other browsers and would have kept the undo history.

I even tried document.execCommand('paste') but that doesn't work at all for inserting text into editable elements.
+1, same issues as mentioned above - currently working on an editor (textarea) with markdown support and e.g. buttons / keyboard shortcuts like cmd+b to make it **bold**.
A very good example is to compare/use the github text editor (github issues, ...).
Make text bold via the menu or [cmd+b] - then try to undo/redo [cmd+z] ... not working in Firefox due to the simple value replace is used as a fallback to `document.execCommand('insertText', false, text)`

Perhaps, after fixing bug 1529884, we could fix this easier than current design. I'll try to fix this in 69.

Not actually working on, but I'd like to fix this later (currently, "beforeinput" event support is more important).

Hello,

I am just dropping by to say that I have referenced this bug report in a Stack Overflow answer.

https://stackoverflow.com/a/58437149/3961271

Probably, I should work on this when I have much time at implementing "beforeinput" event, e.g., when I wait review.

FYI: I've already added WPT for this https://searchfox.org/mozilla-central/source/testing/web-platform/tests/editing/other/exec-command-with-text-editor.tentative.html
And mostly, I have idea where needs to be changed for execCommand. However, I'm still not sure about other related API like queryCommandEnabled etc, but perhaps, not so difficult.

@masayuki

I don't think any changes are necessary for queryCommandEnabled, since all the command-related functions are children of the document object. In Firefox (tested on 69 and 70), document.queryCommandSupported('insertText') (and same for "paste") already return true, indicating that the commands are supported (they just don't work if the cursor is in a textarea or input element... which I would argue is the most common reason to use these commands).

To me it is crazy that HTML5 doesn't simply have an attribute, for the textarea element, that enables your ability to type tabs into textareas.

Thankfully, in Chrome you can accomplish this sensibly using document.execCommand:
https://stackoverflow.com/a/52918135/217867

However, in Firefox, I'm having to write code that "changes the whole value of the textarea" in order to simply insert a tab, and this technique breaks undo-functionality. Imagine textareas with huge amounts of text in them; its crazy to have to change the whole textareaElement.value just to insert a '\t'.

Masayuki, I too think it is a bit strange that the .execCommand method is just on the document object. You'd think this would be a prototype method on "all html elements that can receive inputted text".

However, I'll say this, given the way it is, you should simply look at which element has focus, at the time that document.execCommand is called, and take note of the caret position within that focused element, and then write the text at that caret location. This seems to be how Chromium is accomplishing this.

I concur, this is really irritating if you want to insert text into a textarea and preserve the ability to undo/redo. document.execCommand seems to be the only way to do that.

I agree that this is an important issue. As far as I can tell, it's currently impossible to implement a decent (<textarea>-based) text editor that works in Firefox. One must either make editor controls unintuitive and annoying by having them never replace selected text (so the user can't irrevocably delete text by accidentally hitting the wrong control), or let them be mines waiting for the user to step on them. Neither option is good. And either way, not being able to undo/redo is of course very annoying for the user.

Resetting assignee which I don't work on in this several months.

Assignee: masayuki → nobody
Assignee: nobody → masayuki
Severity: normal → S3
Status: NEW → ASSIGNED
Priority: -- → P3

Editor command should be handled in same command table between ExecCommand
and the other related methods. However, currently, only ExecCommand does
optimized things. For using same logic in the other methods, the code should
be in an independent stack class.

Depends on D108566

This will guarantee that when <input> or <textarea> is in contenteditable,
execCommand and the other related methods work with same command class
instance and same command context (in this case, it's editor instance).

Depends on D108567

For making execCommand and related methods with <input> and <textarea>
even if they are in contenteditable and focused, command should be handled
in active editor (focused editor if in foreground window and tab).

However, some commands should be handled by HTMLEditor even if an TextEditor
has focus. Therefore, this patch adds new enum class which have 3 state into
InternalCommandData and makes AutoEditorCommandTarget consider it with the
enum class.

Note that the new failures about contentReadOnly command will be fixed by
a following patch.

Depends on D108568

Currently, Document checks it only with whether the document is editable
or not. Only with this check, execCommand and the other related methods
work only when there is contenteditable.

Therefore, this patch makes it to check whether the target is editable or not
with target editor.

Depends on D108569

Although this command is supported only by Gecko, we shouldn't stop supporting
this unless we know the usage in the wild. Therefore, this patch adds the
handling code for TextEditor too.

Depends on D108570

Those command handlers just check whether the given editor is an HTMLEditor
or not. Therefore, we should make them check whether the given editor is
a single line editor or not instead.

Depends on D108571

Attachment #9209364 - Attachment description: Bug 1220696 - part 5: Support "contentReadOnly" command in `<input>` and `<textarea>` r=smaug! → Bug 1220696 - part 5: Support "contentReadOnly" and "getHTML" commands in `<input>` and `<textarea>` r=smaug!
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/ade95bf58db4
part 0: Update `exec-command-with-text-editor.tentative.html` for testing the following patches more r=smaug
https://hg.mozilla.org/integration/autoland/rev/f165f45f8e1a
part 1: Split off the code considering `EditorCommand` in `Document::ExecCommand` r=smaug
https://hg.mozilla.org/integration/autoland/rev/63ec6c3d05fd
part 2: Make related methods of `Document::ExecCommand` use `AutoEditorCommandTarget` r=smaug
https://hg.mozilla.org/integration/autoland/rev/a4e90de9355d
part 3: Make `AutoEditorCommandTarget` consider command handling editor with command and focused `TextEditor` or `HTMLEditor` r=smaug
https://hg.mozilla.org/integration/autoland/rev/07012ed7a30e
part 4: Make `Document` consider whether the target is editable or not-editable with target editor r=smaug
https://hg.mozilla.org/integration/autoland/rev/5e2f62a11d95
part 5: Support "contentReadOnly" and "getHTML" commands in `<input>` and `<textarea>` r=smaug
https://hg.mozilla.org/integration/autoland/rev/1ad2d5c0f253
part 6: Support `insertLineBreak` and `insertParagraphSeparator` commands in `<textarea>` r=smaug
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/28199 for changes under testing/web-platform/tests
Whiteboard: [specification][type:bug] → [specification][type:bug], [wptsync upstream]
Upstream PR was closed without merging
Flags: needinfo?(masayuki)
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/2ebe50bc5498
part 0: Update `exec-command-with-text-editor.tentative.html` for testing the following patches more r=smaug
https://hg.mozilla.org/integration/autoland/rev/b6bd98c80b7b
part 1: Split off the code considering `EditorCommand` in `Document::ExecCommand` r=smaug
https://hg.mozilla.org/integration/autoland/rev/c1739e11849e
part 2: Make related methods of `Document::ExecCommand` use `AutoEditorCommandTarget` r=smaug
https://hg.mozilla.org/integration/autoland/rev/4580967d47ff
part 3: Make `AutoEditorCommandTarget` consider command handling editor with command and focused `TextEditor` or `HTMLEditor` r=smaug
https://hg.mozilla.org/integration/autoland/rev/40020a5f44e1
part 4: Make `Document` consider whether the target is editable or not-editable with target editor r=smaug
https://hg.mozilla.org/integration/autoland/rev/c8b24f26ba71
part 5: Support "contentReadOnly" and "getHTML" commands in `<input>` and `<textarea>` r=smaug
https://hg.mozilla.org/integration/autoland/rev/afd35aefdf51
part 6: Support `insertLineBreak` and `insertParagraphSeparator` commands in `<textarea>` r=smaug

The behavior will be:

  • When <input> or <textarea> has focus, most commands of execCommand, etc are handled by it.
  • When they are disabled, they just do nothing, and also the document's editor does nothing too.
  • The commands which set document state like default paragraph separator, using CSS or HTML inline-style elements, they are always handled by the document's HTML editor, even when <input> or <textarea> has focus.
  • The commands which set inline or block style around selection, they do nothing when <input> or <textarea> has focus.

Please note Bug 1695659 which may have an affect on this bug as well (execCommand on page with iframes).

(In reply to erosman from comment #59)

Please note Bug 1695659 which may have an affect on this bug as well (execCommand on page with iframes).

clipboard commands are handled only with focused descendant if an <iframe> has focus, and won't be go back to parent document from a child document if the execCommand is called in a child document.

Upstream PR was closed without merging
Upstream PR merged by moz-wptsync-bot
Regressions: 1702670
Regressions: 1701348

Hi :masayuki!

I'm currently trying to work out how to document this change on MDN. Does the following description and requirements sound accurate, or is it not that simple?

"
Release note:

From Firefox 89 onwards, the content of <input> and <textarea> elements that have been put into designMode or had contentEditable set on them can now be manipulated using Document.execCommand() commands, preserving edit history and providing parity with other browsers.

Add browser compat data to make this change clear on:

Do we need to say exactly which execCommand() commands were affected, and if so, do you have a definitive list?

Thanks.

Flags: needinfo?(masayuki)

Thank you for updating the doucments!

From Firefox 89 onwards, the content of <input> and <textarea> elements that have been put into designMode or had contentEditable set on them can now be manipulated using Document.execCommand() commands

Now, they don't require designMode nor contenteditable. They were required 88 and earlier ("have been/had"'s nuance is unclear to me in this case).

Do we need to say exactly which execCommand() commands were affected, and if so, do you have a definitive list?

I don't think it's required because I don't check the other browsers' behavior so deeply. If you want to do it, you can check our source code:
https://searchfox.org/mozilla-central/rev/1df3b4b4d27d413675fb66f375230cb25d636450/dom/base/Document.cpp#4414,4420,4422,4428,4430,4436,4438,4444,4446,4452,4454,4460,4462,4468,4470,4476,4478,4484,4486,4492,4494,4500,4502,4508,4510,4516,4518,4524,4526,4531,4533,4539,4541,4547,4549,4555,4557,4563,4565,4571,4573,4579,4581,4587,4589,4595,4597,4603,4605,4611,4613,4619,4621,4627-4630,4632,4638,4640,4646-4649,4651,4657,4659,4665,4667,4673,4675,4681,4683,4689,4691,4697,4699,4705,4707,4713,4715,4721,4723,4729,4731,4737,4739,4745,4747,4753,4755,4761,4763,4769,4771,4777,4779,4785,4787,4793,4795,4801,4803,4809,4811,4817

  • CommandOnTextEditor::Enabled means the command is handled (and consumed) by <input> or <textarea> if and only if one has focus.
  • CommandOnTextEditor::Disabled means the command is not handled and consumed by <input> or <textarea> if and only if one has focus (i.e., even if one is in contenteditable, it's not handled by the HTML editor too)
  • CommandOnTextEditor::FallThrough means the command is handled by contenteditable even if <input> or <textarea> has focus. In this case, the focused one does not need to be in contenteditable. It affects to HTMLEditor instance which manage all contenteditables in the document.
Flags: needinfo?(masayuki)

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #66)

Thank you for updating the doucments!

From Firefox 89 onwards, the content of <input> and <textarea> elements that have been put into designMode or had contentEditable set on them can now be manipulated using Document.execCommand() commands

Now, they don't require designMode nor contenteditable. They were required 88 and earlier ("have been/had"'s nuance is unclear to me in this case).

Ah, great, thanks :masayuki!

I understand this better now, and after doing a bit of testing. I'm going to update the rel note text to:

"From Firefox 89 onwards, the content of <input> and <textarea> elements can now be manipulated using Document.execCommand() commands by default, preserving edit history and providing parity with other browsers, without contentEditable or any lengthy workarounds required ({{bug(1220696)}})."

I'll set this to dev-doc-complete now, but please speak up if you think the MDN notes need changing. Thanks!

See https://github.com/mdn/content/issues/4311 for the full details of what was done for the docs.

Thanks, that looks good to me!

You need to log in before you can comment on or make changes to this bug.