1352144 - document.execCommand("inserttext", false, "") should work

Status: RESOLVED FIXED

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

Description

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Behavior per spec and Chrome is to just delete the selection.  We already do this for insertHTML.

Comment 1

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Bug 1352144 - document.execCommand("inserttext", false, "") should work

Behavior per spec and Chrome is to just delete the selection.  We
already do this for insertHTML.

Review commit: https://reviewboard.mozilla.org/r/125158/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/125158/

Comment 2

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Comment on attachment 8853064 [details]
Bug 1352144 - document.execCommand("inserttext", false, "") should work

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/125158/diff/1-2/

Comment 3

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Comment on attachment 8853064 [details]
Bug 1352144 - document.execCommand("inserttext", false, "") should work

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/125158/diff/2-3/

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8853064 [details]
Bug 1352144 - document.execCommand("inserttext", false, "") should work

https://reviewboard.mozilla.org/r/125158/#review128398

::: editor/libeditor/HTMLEditRules.cpp
(Diff revision 3)
> -  if (inString->IsEmpty() && aAction != EditAction::insertIMEText) {
> -    // HACK: this is a fix for bug 19395
> -    // I can't outlaw all empty insertions
> -    // because IME transaction depend on them
> -    // There is more work to do to make the
> -    // world safe for IME.
> -    *aCancel = true;
> -    *aHandled = false;
> -    return NS_OK;
> -  }
> -

So, removing this block from HTMLEditRules::WillInsertText(), it's allowed to input empty string even from keyboard from the view point of HTMLEditRules.

On the other hand, HTMLEditor::HandleKeyPressEvent() ignores keypress events whose mCharCode is 0 and not a special key.

So, perhaps, this block has been a dead code actually. However, I worry about this causing a regression.

Could you add MOZ_ASSERT(!aString.IsEmpty() || aAction != EditAction::insertText); to HTMLEditor::TypedText()?

Comment 5

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Comment on attachment 8853064 [details]
Bug 1352144 - document.execCommand("inserttext", false, "") should work

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/125158/diff/3-4/

Comment 6

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Comment on attachment 8853064 [details]
Bug 1352144 - document.execCommand("inserttext", false, "") should work

https://reviewboard.mozilla.org/r/125158/#review128398

> So, removing this block from HTMLEditRules::WillInsertText(), it's allowed to input empty string even from keyboard from the view point of HTMLEditRules.
> 
> On the other hand, HTMLEditor::HandleKeyPressEvent() ignores keypress events whose mCharCode is 0 and not a special key.
> 
> So, perhaps, this block has been a dead code actually. However, I worry about this causing a regression.
> 
> Could you add MOZ_ASSERT(!aString.IsEmpty() || aAction != EditAction::insertText); to HTMLEditor::TypedText()?

aAction here is ETypingAction, not EditAction -- I guess you mean eTypedText instead of EditAction::insertText?

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e702b7444ce8
document.execCommand("inserttext", false, "") should work r=masayuki

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e702b7444ce8