Closed Bug 948839 Opened 11 years ago Closed 11 years ago

Drag and Drop broken

Categories

(Core :: DOM: Events, defect)

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.
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.
Attached patch Patch (deleted) — Splinter Review
Attachment #8346419 - Flags: review?(bugs)
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+
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
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.
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.

Attachment

General

Created:
Updated:
Size: