1616539 - Cannot drag & drop an image file to contenteditable from Windows10 Explorer even if insertion point is indicated

Status: VERIFIED FIXED

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

Description

[Alice0775 White]

Attachment: contenteditable.html

Reproducible : always

Steps To Reproduce:

1. Save attached html
2. Open the html file and Don't click contentarea
   ---- now, there are Windows Explorer and Firefox window. And Firefox window is focused.
3. Drag & drop an image file from Windows Explorer to contenteditable element

Actual Results:
Insertion point indicated when dragging over contenteditable as expected.
However, nothing is dropped. No image display in the contentaditable.

Expected Results:
Image is inserted and display in the contentaditable.

OR

Insertion point should not display. And feedback mouse cursor should be "no-drop".

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=cae6a2e1838b522f7e940d41db2b61299f9e2b18&tochange=f2637cbcc2ca75be447689fd66baa0e5bfd07c66

Comment 1

[John Dai[:jdai]]

Hi masayuki, According to the regression window, could you take a look? Thank you.

Comment 2

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

Oh, is this the regression of bug 1597829? I realized this bug, but I thought this is traditional bug.

Comment 3

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

Attachment: Bug 1616539 - Make `TextEditor::OnDrop()` compare newly focused element with active editing host r=m_kato!

Currently, it checks whether proper element gets focus as expected with
nsFocusManager::GetFocusedElement(), but it returns globally focused element.
I.e., it may return different document's element or nullptr if application
itself is inactive.

The purpose of the focus check is, HTMLEditor can modify contents only in
active editing host. Therefore, comparing with
HTMLEditor::GetActiveEditingHost() is better and simpler for here.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/81f420f057e4
Make `TextEditor::OnDrop()` compare newly focused element with active editing host r=m_kato

Comment 5

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/81f420f057e4

Comment 6

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

Comment on attachment 9128000 [details]
Bug 1616539 - Make TextEditor::OnDrop() compare newly focused element with active editing host r=m_kato!

Beta/Release Uplift Approval Request

• User impact if declined: User cannot drop image file from another application (this bug) and cannot drag and drop images in https://www.toepferei-koerner.de/dragdroptest.html (bug 1616509). The latter must be more major situation than the former.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: More important case (bug 1616509):

1. Go to https://www.toepferei-koerner.de/dragdroptest.html
2. Drag an image from the first block.
3. Drop it into the below editor.

And this bug's case:

1. Go to attachment 9127531 [details]
2. Activate another application like a filer
3. Drag an image file
4. Drop onto the (inactive) Firefox window which opens the attachment.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The focus move check was added by the regressed bug, but it was comparing with global focused element. However, it should be compared with active element in the document which receive the drop event. Therefore, this patch just makes expected element compared with active editing host of the HTMLEditor which received the drop event.

On the other hand, I don't know how many web apps are broken by this regression and how much DnD in such web apps is important. So, feel free to reject this request if it's too late and feels risky.

• String changes made/needed: none

Comment 7

[Gyula Palko[:gpalko]]

Reproduced the issue on Nightly 75(20200224162717), verified as fixed on Nightly 75(20200225214332) on Win10 x64 and macOS 10.15. Tested with both test cases mentioned in Comment 6.

Comment 8

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9128000 [details]
Bug 1616539 - Make TextEditor::OnDrop() compare newly focused element with active editing host r=m_kato!

I consulted with the webcompat team and they are not aware of other reported issues caused by this bug, so I am not going to take it for our last beta to minimize risk of creating worse regressions than this one. Thanks.