506081 - dataTransfer.setData makes scrollbars draggable off of themselves on dragstart events

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Eli Grey (:sephr)]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US-Hixie; rv:1.9.1.2pre) Gecko/20090723 Shiretoko/3.5.2pre
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US-Hixie; rv:1.9.1.2pre) Gecko/20090723 Shiretoko/3.5.2pre

Scrollbars (and every part of their UI) become draggable akin the effect seen in bug 24024 when dataTransfer.setData is used in a dragstart event that was fired on a scrollbar.

This bug affects any website that uses the CiteDrag JavaScript library ( http://github.com/eligrey/CiteDrag/tree/master ), which includes my blog.

Reproducible: Always

Steps to Reproduce:
1. document.addEventListener("dragstart", function(e) {e.dataTransfer.setData("text/vnd.initech.foobar","")}, false)
2. Drag a part of a scrollbar UI
Actual Results:  
Scrollbar UI part is dragged off itself (copied, not removed).

Expected Results:  
Scrollbar UI part functions normally.


Side-question: Are scrollbars part of the DOM?

Comment 1

[Eli Grey (:sephr)]

Attachment: Testcase

Comment 2

[Eli Grey (:sephr)]

I copied and pasted the wrong bug ID. When I said bug 24024 I meant bug 324024.

Comment 3

[Nochum Sossonko [:Natch]]

Olli, would it be ok if I added a PreventDefault in nsSliderFrame::MouseDown, or would that break something?

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #3)
> Olli, would it be ok if I added a PreventDefault in nsSliderFrame::MouseDown,
> or would that break something?
Why would you want to do that?
Dragging non-native-anonymous scrollbars should be probably possible.
So perhaps better would be to prevent default handling for native anonymous
scroll bars. So add something to nsXULElement::PreHandleEvent.
Hopefully preventing and cancelling dragstart should be enough.

FYI, I can't always reproduce the problem.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #4)
> Hopefully preventing and cancelling dragstart should be enough.
Or something like that. I'm not quite sure what this bug is about, because
I can't really reproduce it easily (Linux/OSX)

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Ok, now I can reproduce

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Attachment: wip

We probably don't want to stop the propagation of all drag events, but
anyway this seems to fix this bug.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Enn, what do you think about this.
This way if dragging starts in a native anon scroll bar, and nothing is set to be
drag data, we don't really start drag process since InvokeDragSessionXXX doesn't
get called.

Comment 9

[[e-mail]]

Comment on attachment 390465 [details] [diff] [review]
patch

>+         aVisitor.mEvent->message == NS_CONTEXTMENU ||
>+         aVisitor.mEvent->message == NS_CONTEXTMENU ||
You're just double-checking that we don't open context menus on scrollbars, right? ;-)

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Uh, how did I do that o_O

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/682ac215c2e2

Comment 13

[Nochum Sossonko [:Natch]]

Olli: do you think this can (and should) go into 1.9.1.x?

Comment 14

[Nochum Sossonko [:Natch]]

Regression from 1.9.0.

Comment 15

[Samuel Sidler (old account; do not CC)]

What bug regressed this?

Comment 16

[Nochum Sossonko [:Natch]]

Regression range:
2008-08-27 works
2008-08-28 broken
Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-08-27&enddate=2008-08-28
->Bug 356295 probably. The other drag crash bug was fixed on 1.9.0 and it still doesn't exhibit this bugs behavior.

Comment 17

[Samuel Sidler (old account; do not CC)]

Olli: Is this worth fixing on 1.9.1? I don't think it blocks anyway, but if you think we should take it, please give it an approval1.9.1.4 request.