Closed Bug 948839 Opened 6 years ago Closed 6 years ago

Drag and Drop broken

Categories

(Core :: DOM: Events, defect, major)

28 Branch
x86_64
Windows 7
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla29
Tracking Status
firefox27 --- unaffected
firefox28 + verified
firefox29 - verified

People

(Reporter: alice0775, Assigned: masayuki)

References

Details

(Keywords: regression)

Attachments

(1 file)

Steps To Reproduce #1:
1. Select text
2. Drag the selected text
3. Drop on <input> or <textarea>

Actual Results:
Search by default search engine

Expected Results:
Dragged test should be copied to input/textarea

Steps To Reproduce #2:
1. Type some text in <input> or <textarea> 
2. Select text inside the <input> or <textarea> 
3. Drag the selected text
4. Drop on the <input> or <textarea> 

Actual Results:
Search by default search engine

Expected Results:
Dragged test should be moved


Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/06b3a7aea2c0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20131208154802
Bad:
http://hg.mozilla.org/mozilla-central/rev/551efcc4de6f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20131208185601
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=06b3a7aea2c0&tochange=551efcc4de6f

Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/046cf0c4f858
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20131207204100
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/2aaff66026ba
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20131208075200
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=046cf0c4f858&tochange=2aaff66026ba
I wonder that no automatic test is provided.
Steps To Reproduce #3:
1. Drag alink
2. Drop on the <input> or <textarea> 

Actual Results:
Link opened in current tab

Expected Results:
Link url should be copied to input/textarea
Steps To Reproduce #4:
1. Drag a file from explorer
2. Drop on the <input type="file"> ("Browse.." button or "No file selected" label)

Actual Results:
file opened in current tab

Expected Results:
file path should be copied to <input type="file">
I've checked the removed code again.

http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsDOMEvent.cpp?rev=5721f317bab1#442
> 442     // Need to set an extra flag for drag events.
> 443     if (mEvent->eventStructType == NS_DRAG_EVENT && IsTrusted()) {
> 444       nsCOMPtr<nsINode> node = do_QueryInterface(mEvent->currentTarget);
> 445       if (!node) {
> 446         nsCOMPtr<nsPIDOMWindow> win = do_QueryInterface(mEvent->currentTarget);
> 447         if (win) {
> 448           node = win->GetExtantDoc();
> 449         }
> 450       }
> 451       if (node && !nsContentUtils::IsChromeDoc(node->OwnerDoc())) {
> 452         mEvent->mFlags.mDefaultPreventedByContent = true;
> 453       }
> 454     }

Perhaps, we misunderstand this code. mDefaultPreventedByContent is wrong name. mDefaultPreventedByContent became true when the event is consumed *on* content.

I guess that we should add mDefaultPreventedOnContent to the WidgetDragEvent and recover the code with it. How do you think, smaug?
Flags: needinfo?(bugs)
hmm, what is calling preventDefault()? The original idea for that code was that if content calls
preventDefault() we handle that case differently.
Does chrome add drag listener to content?

But
Flags: needinfo?(bugs)
er,

But for FF28 adding mDefaultPreventedOnContent to WidgetDragEvent should be ok.

And sorry, I should have looked at this more when I suggested using mDefaultPreventedByContent.
Duplicate of this bug: 949130
The callers of preventDefault() are:
http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/base/nsEditorEventListener.cpp?rev=c9879b47c32c#624
http://mxr.mozilla.org/mozilla-central/source/layout/forms/nsFileControlFrame.cpp?rev=2ae6663f558c#193

I think that they should mark special flag for dragover event themselves. However, this mechanism may be used by others including add-ons. So, I believe that we cannot take this approach for fixing this.
(In reply to Alice0775 White from comment #1)
> I wonder that no automatic test is provided.

It's difficult to test of the behavior of DnD especially at dragging special object...
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
(In reply to Olli Pettay [:smaug] from comment #5)
> hmm, what is calling preventDefault()? The original idea for that code was
> that if content calls
> preventDefault() we handle that case differently.
> Does chrome add drag listener to content?

Yes, the callers of preventDefault() is C++ handler (and can be chrome's event listener). Even in such cases, if the currentTarget of the dragover event belongs content, mDefaultPreventedByContent was set true.
Comment on attachment 8346419 [details] [diff] [review]
Patch

oops, I forgot to change IPC.
Attachment #8346419 - Flags: review?(bugs) → review-
Comment on attachment 8346419 [details] [diff] [review]
Patch

Sorry for the spam. There is no code for WidgetDragEvent in nsGUIEventIPC.h :-(
Attachment #8346419 - Flags: review- → review?(bugs)
Attachment #8346419 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/23bbaca5db4f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Duplicate of this bug: 948684
Duplicate of this bug: 948864
Masayuki, we want this to Aurora too, right?
Absolutely. But I want to wait a couple of days for confirming no regression on m-c.
Duplicate of this bug: 950555
Comment on attachment 8346419 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #):
bug 930374. It removed necessary code accidentally.

User impact if declined:
User cannot drop files/URLs on <input type="file">, <input type="text">, etc. The dropped item always causes open in the tab.

Testing completed (on m-c, etc.):
Landed on the last week.

Risk to taking this patch (and alternatives if risky):
No idea because this patch just restores the removed code.

String or IDL/UUID changes made by this patch:
No.
Attachment #8346419 - Flags: approval-mozilla-aurora?
Attachment #8346419 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Alice, please confirm this is working for you now in Firefox 28 and 29.
Flags: needinfo?(alice0775)
This broke for me upgrading to 28.0a2. I can't find a nightly regression window, but I can reproduce it with Aurora on two different computers. This one is using 20131219004003 and the other is still on 20131214004001. This one was still on 27.0a2 20131206004003 and was working fine, broke after updating.

This should have the dataloss keyword, I have been hitting this while composing messages for work, needing to move some text around, and then I'm navigated away. When I hit back the form data is gone :(
(In reply to Majken Connor [Kensie] from comment #24)
> This broke for me upgrading to 28.0a2. I can't find a nightly regression
> window, but I can reproduce it with Aurora on two different computers. This
> one is using 20131219004003 and the other is still on 20131214004001. This
> one was still on 27.0a2 20131206004003 and was working fine, broke after
> updating.
> 
> This should have the dataloss keyword, I have been hitting this while
> composing messages for work, needing to move some text around, and then I'm
> navigated away. When I hit back the form data is gone :(

The nightly build of Aurora28.0a2 is not released yet.
Maybe it avairable today's nightly(2013-Dec-20).
Flags: needinfo?(alice0775)
Correcting:
The nightly build of Aurora28.0a2 which is included this patch is not released yet.
Maybe it avairable today's nightly(2013-Dec-20).
And also you can try latest tinderbox build(include this patch) in http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-aurora-win32/
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #23)
> Alice, please confirm this is working for you now in Firefox 28 and 29.

I cannot reproduce the problem with STR#1-#4.
And I verified that the problem was fixed.

http://hg.mozilla.org/mozilla-central/rev/5c7fa2bfea8b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20131219030202
http://hg.mozilla.org/releases/mozilla-aurora/rev/40c4cb09bce5
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20131220004002
(In reply to Majken Connor [Kensie] from comment #24)
> This broke for me upgrading to 28.0a2. I can't find a nightly regression
> window, but I can reproduce it with Aurora on two different computers. This
> one is using 20131219004003 and the other is still on 20131214004001. This
> one was still on 27.0a2 20131206004003 and was working fine, broke after
> updating.

This should be fixed in Firefox Nightly 29.0a1 as of 2013-12-13 and Firefox Aurora 28 as of 2013-12-20. I'm marking this verified fixed based on Alice's testing but please do reopen if this isn't working for you after updating to today's Aurora.
Status: RESOLVED → VERIFIED
Thanks, just updated, it is working for me as well.
You need to log in before you can comment on or make changes to this bug.