Closed Bug 1352144 Opened 6 years ago Closed 6 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: ayg, Assigned: ayg)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Behavior per spec and Chrome is to just delete the selection.  We already do this for insertHTML.
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()?
Attachment #8853064 - Flags: review?(masayuki) → review+
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?
Priority: -- → P3
Pushed by ayg@aryeh.name:
https://hg.mozilla.org/integration/autoland/rev/e702b7444ce8
document.execCommand("inserttext", false, "") should work r=masayuki
https://hg.mozilla.org/mozilla-central/rev/e702b7444ce8
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.