Closed Bug 1504910 Opened 6 years ago Closed 6 years ago

Drag and drop in same editor should cause "input" event twice

Categories

(Core :: DOM: Editor, enhancement)

Product:
Core
Component:
DOM: Editor
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla65
Tracking Flags:
Tracking Status
firefox65 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: parity-chrome, parity-safari)

Attachments

(3 files)

Bug 1504910 - part 1: Clean up methods which are called by TextEditor::OnDrop()
47 bytes, text/x-phabricator-request
Details | Review
Bug 1504910 - part 2: Make TextEditor::InsertTextAt() and HTMLEditor::DoInsertHTMLWithContext() share preparation code before inserting content
47 bytes, text/x-phabricator-request
Details | Review
Bug 1504910 - part 3: Make TextEditor::OnDrop() remove selected content before inserting dropped content and fire "input" event before inserting first content
47 bytes, text/x-phabricator-request
Details | Review 
When drag and drop occurs in same editor, we should fire "input" event twice. One is for "deleteByDrag" and the other is for "insertFromDrop".

This is important for web apps which want to cancel only one of them when we implement "beforeinput".
TextEditor::OnDrop() calls TextEditor::InsertFromDataTransfer() or
HTMLEditor::InsertFromDataTransfer() and they are called only by
TextEditor::OnDrop().

TextEditor::InsertFromDataTransfer() calls only TextEditor::InsertTextAt()
and TextEditor::InsertTextAt() calls only TextEditor::InsertTextAsSubAction() if
insertion point is nullptr.  Therefore, if the callers sets nullptr, they
should call TextEditor::InsertTextAsSubAction() directly.  Then, we can
make TextEditor::InsertTextAt() require non-nullptr insertion point.

HTMLEditor::InsertFromDataTransfer() calls HTMLEditor::InsertObject(),
HTMLEditor::DoInsertHTMLWithContext() or TextEditor::InsertTextAt().

HTMLEditor::InsertObject() calls HTMLEditor::DoInsertHTMLWithContext()
directly or via BlobReader (in this case, calls asynchronously).

Unfortunately both HTMLEditor::InsertObject() and
HTMLEditor::DoInsertHTMLWithContext() are called by
HTMLEditor::InsertFromTransferable() which is paste event handler.  Therefore,
we cannot make them require non-nullptr insertion point, though, anyway,
they cannot become simpler even if we could do that.

This patch marks them as MOZ_CAN_RUN_SCRIPT as far as possible and
makes them take |const EditorDOMPoint&| for insertion point.
Only TextEditor::InsertTextAt() and HTMLEditor::DoInsertHTMLWithContext()
removes selected content before pasting from clipboard or inserting dropped
content, and they do same things.  Therefore, they can share the code with
a helper method.

Note that this makes each root caller won't return NS_ERROR_EDITOR_DESTROYED
since the destruction is expected by web app.
Both Chrome and Safari dispatches 2 sets of "beforeinput" and "input" events
when user drag and drop content in same editor.  One is "deleteByDrag" and
the other is "insertFromDrop".  We need to follow same behavior since
it should be able to cancel only "deleteByDrag" or "insertFromDrop" when
we implement "beforeinput" event.

HTMLEditor::InsertObject() may handle asynchronously.  And we don't need to
do anything additionally for HTMLEditor.  Therefore, TextEditor::OnDrop()
can delete selection before inserting dropped content by itself.

Therefore, this patch makes TextEditor::OnDrop() call
TextEditor::PrepareToInsertContent() and EditorBase::FireInputEvent() by
itself.

Unfortunately, it seems that we cannot write automated tests for this.
Even if using synthesizeMouse() of EventUtils, synthesizing mouse events
won't generate "dragover" nor "drop" events.  Perhaps, like EventUtils.js,
we need to do something with drag service, but it means that we cannot
emulate drag and drop in an editor.
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/30487cc4d50c
part 1: Clean up methods which are called by TextEditor::OnDrop() r=m_kato
https://hg.mozilla.org/integration/autoland/rev/7e18b2160df5
part 2: Make TextEditor::InsertTextAt() and HTMLEditor::DoInsertHTMLWithContext() share preparation code before inserting content r=m_kato
https://hg.mozilla.org/integration/autoland/rev/01cd5b54d3ee
part 3: Make TextEditor::OnDrop() remove selected content before inserting dropped content and fire "input" event before inserting first content r=m_kato
https://hg.mozilla.org/mozilla-central/rev/30487cc4d50c
https://hg.mozilla.org/mozilla-central/rev/7e18b2160df5
https://hg.mozilla.org/mozilla-central/rev/01cd5b54d3ee
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox65: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Regressions: 1556235
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: