Changeset 55977 in webkit


Ignore:
Timestamp:
Mar 14, 2010 5:01:57 PM (14 years ago)
Author:
oliver@apple.com
Message:

2010-03-14 Oliver Hunt <​oliver@apple.com>

Reviewed by Darin Adler.

REGRESSION(r53287): drop event is not fired if dataTransfer.dropEffect is not explicitly set
​https://bugs.webkit.org/show_bug.cgi?id=36095

Correct this test to cover the somewhat bizarre "correct" handling
of undefined dropEffect

  • fast/events/drag-and-drop-expected.txt:
  • fast/events/drag-and-drop.html:

2010-03-14 Oliver Hunt <​oliver@apple.com>

Reviewed by Darin Adler.

REGRESSION(r53287): drop event is not fired if dataTransfer.dropEffect is not explicitly set
​https://bugs.webkit.org/show_bug.cgi?id=36095

The issue here is that while dropEffect is meant to be initialized
to "none", the behaviour of the drag is differs between dragEffect
not being set and dragEffect being explicitly set to "none"

This patch corrects this behaviour by making Clipboard distinguish
between the initial "none" value of dropEffect and an explicit "none".
This alone is insufficient for correct behaviour, we also need to
resurrect the removed defaultOperationForDrag function, but we now
use the function only when dragEffect is uninitialized. There are a
few tweaks to the behaviour of the defaultOperationForDrag as well
to ensure exactly the same set of outcomes for all cases that we
may hit it.

  • dom/Clipboard.cpp: (WebCore::Clipboard::Clipboard): (WebCore::dragOpFromIEOp): (WebCore::Clipboard::destinationOperation):
  • dom/Clipboard.h: (WebCore::Clipboard::dropEffect): (WebCore::Clipboard::dropEffectIsUninitialized):
  • page/DragController.cpp: (WebCore::defaultOperationForDrag): (WebCore::DragController::tryDHTMLDrag):
Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r55950 r55977  
     12010-03-14  Oliver Hunt  <oliver@apple.com>
     2
     3        Reviewed by Darin Adler.
     4
     5        REGRESSION(r53287): drop event is not fired if dataTransfer.dropEffect is not explicitly set
     6        https://bugs.webkit.org/show_bug.cgi?id=36095
     7
     8        Correct this test to cover the somewhat bizarre "correct" handling
     9        of undefined dropEffect
     10
     11        * fast/events/drag-and-drop-expected.txt:
     12        * fast/events/drag-and-drop.html:
     13
    1142010-03-12  Shu Chang  <chang.shu@nokia.com>
    215

  • trunk/LayoutTests/fast/events/drag-and-drop-expected.txt

    r53287 r55977  
    1212PASS event.dataTransfer.dropEffect is "move"
    1313PASS event.dataTransfer.dropEffect is "link"
    14 PASS event.dataTransfer.dropEffect is "none"
     14PASS event.dataTransfer.dropEffect is "copy"
    1515
    1616When effectAllowed == "undefined"
    … …  
    2525PASS event.dataTransfer.dropEffect is "link"
    2626PASS event.dataTransfer.effectAllowed is "uninitialized"
    27 PASS event.dataTransfer.dropEffect is "none"
     27PASS event.dataTransfer.dropEffect is "copy"
    2828
    2929When effectAllowed == "none"
    … …  
    4141PASS event.dataTransfer.dropEffect is "move"
    4242PASS event.dataTransfer.dropEffect is "link"
    43 PASS event.dataTransfer.dropEffect is "none"
     43PASS event.dataTransfer.dropEffect is "copy"
    4444
    4545When effectAllowed == "copy"
    … …  
    4949PASS event.dataTransfer.dropEffect is "none"
    5050PASS event.dataTransfer.dropEffect is "none"
    51 PASS event.dataTransfer.dropEffect is "none"
     51PASS event.dataTransfer.dropEffect is "copy"
    5252
    5353When effectAllowed == "move"
    … …  
    5757PASS event.dataTransfer.dropEffect is "move"
    5858PASS event.dataTransfer.dropEffect is "none"
    59 PASS event.dataTransfer.dropEffect is "none"
     59PASS event.dataTransfer.dropEffect is "move"
    6060
    6161When effectAllowed == "link"
    … …  
    6565PASS event.dataTransfer.dropEffect is "none"
    6666PASS event.dataTransfer.dropEffect is "link"
    67 PASS event.dataTransfer.dropEffect is "none"
     67PASS event.dataTransfer.dropEffect is "link"
    6868
    6969When effectAllowed == "copyMove"
    … …  
    7373PASS event.dataTransfer.dropEffect is "move"
    7474PASS event.dataTransfer.dropEffect is "none"
    75 PASS event.dataTransfer.dropEffect is "none"
     75PASS event.dataTransfer.dropEffect is "move"
    7676
    7777When effectAllowed == "copyLink"
    … …  
    8181PASS event.dataTransfer.dropEffect is "none"
    8282PASS event.dataTransfer.dropEffect is "link"
    83 PASS event.dataTransfer.dropEffect is "none"
     83PASS event.dataTransfer.dropEffect is "copy"
    8484
    8585When effectAllowed == "linkMove"
    … …  
    8989PASS event.dataTransfer.dropEffect is "move"
    9090PASS event.dataTransfer.dropEffect is "link"
    91 PASS event.dataTransfer.dropEffect is "none"
     91PASS event.dataTransfer.dropEffect is "move"
    9292
    9393When effectAllowed == "dummy"
    … …  
    102102PASS event.dataTransfer.dropEffect is "link"
    103103PASS event.dataTransfer.effectAllowed is "uninitialized"
    104 PASS event.dataTransfer.dropEffect is "none"
     104PASS event.dataTransfer.dropEffect is "copy"
    105105
    106106TEST COMPLETE

  • trunk/LayoutTests/fast/events/drag-and-drop.html

    r53287 r55977  
    109109            chosenEffectAllowed = "uninitialized";
    110110        }
    111        
     111        expected = dropEffectElem.options[dropEffectElem.selectedIndex].value;
     112        if (chosenDropEffect == "dummy") {
     113            switch (chosenEffectAllowed) {
     114                case "undefined":
     115                case "copyLink":
     116                case "uninitialized":
     117                case "all":
     118                    expected = "copy";
     119                    break;
     120                case "copyMove":
     121                case "move":
     122                case "linkMove":
     123                    expected = "move";
     124                    break;
     125                default:
     126                    expected = chosenEffectAllowed;
     127            }
     128        }
    112129        if (isDropEffectAllowed(chosenDropEffect, chosenEffectAllowed))
    113             shouldBeEqualToString('event.dataTransfer.dropEffect', dropEffectElem.options[dropEffectElem.selectedIndex].value);
     130            shouldBeEqualToString('event.dataTransfer.dropEffect', expected);
    114131        else
    115132            shouldBeEqualToString('event.dataTransfer.dropEffect', 'none');
    … …  
    127144        if (chosenDropEffect == "link" && ["link", "copyLink", "linkMove", "uninitialized", "all"].indexOf(allowedDropEffect) != -1)
    128145            return true;
     146        if (chosenDropEffect == "dummy" && ["copy", "link", "move", "copyLink", "copyMove", "linkMove", "uninitialized", "all"].indexOf(allowedDropEffect) != -1)
     147            return true;
    129148        return false;
    130149    }

  • trunk/WebCore/ChangeLog

    r55976 r55977  
     12010-03-14  Oliver Hunt  <oliver@apple.com>
     2
     3        Reviewed by Darin Adler.
     4
     5        REGRESSION(r53287): drop event is not fired if dataTransfer.dropEffect is not explicitly set
     6        https://bugs.webkit.org/show_bug.cgi?id=36095
     7
     8        The issue here is that while dropEffect is meant to be initialized
     9        to "none", the behaviour of the drag is differs between dragEffect
     10        not being set and dragEffect being explicitly set to "none"
     11
     12        This patch corrects this behaviour by making Clipboard distinguish
     13        between the initial "none" value of dropEffect and an explicit "none".
     14        This alone is insufficient for correct behaviour, we also need to
     15        resurrect the removed defaultOperationForDrag function, but we now
     16        use the function only when dragEffect is uninitialized.  There are a
     17        few tweaks to the behaviour of the defaultOperationForDrag as well
     18        to ensure exactly the same set of outcomes for all cases that we
     19        may hit it.
     20
     21        * dom/Clipboard.cpp:
     22        (WebCore::Clipboard::Clipboard):
     23        (WebCore::dragOpFromIEOp):
     24        (WebCore::Clipboard::destinationOperation):
     25        * dom/Clipboard.h:
     26        (WebCore::Clipboard::dropEffect):
     27        (WebCore::Clipboard::dropEffectIsUninitialized):
     28        * page/DragController.cpp:
     29        (WebCore::defaultOperationForDrag):
     30        (WebCore::DragController::tryDHTMLDrag):
     31
    1322010-03-14  Antti Koivisto  <koivisto@iki.fi>
    233

  • trunk/WebCore/dom/Clipboard.cpp

    r53301 r55977  
    3737Clipboard::Clipboard(ClipboardAccessPolicy policy, bool isForDragging)
    3838    : m_policy(policy)
    39     , m_dropEffect("none")
     39    , m_dropEffect("uninitialized")
    4040    , m_effectAllowed("uninitialized")
    4141    , m_dragStarted(false)
    … …  
    111111{
    112112    DragOperation op = dragOpFromIEOp(m_dropEffect);
    113     ASSERT(op == DragOperationCopy || op == DragOperationNone || op == DragOperationLink || op == DragOperationGeneric || op == DragOperationMove);
     113    ASSERT(op == DragOperationCopy || op == DragOperationNone || op == DragOperationLink || op == DragOperationGeneric || op == DragOperationMove || op == DragOperationEvery);
    114114    return op;
    115115}

  • trunk/WebCore/dom/Clipboard.h

    r54368 r55977  
    4444        bool isForDragging() const { return m_forDragging; }
    4545
    46         String dropEffect() const { return m_dropEffect; }
     46        String dropEffect() const { return dropEffectIsUninitialized() ? "none" : m_dropEffect; }
    4747        void setDropEffect(const String&);
     48        bool dropEffectIsUninitialized() const { return m_dropEffect == "uninitialized"; }
    4849        String effectAllowed() const { return m_effectAllowed; }
    4950        void setEffectAllowed(const String&);

  • trunk/WebCore/page/DragController.cpp

    r55974 r55977  
    486486}
    487487
     488static DragOperation defaultOperationForDrag(DragOperation srcOpMask)
     489{
     490    // This is designed to match IE's operation fallback for the case where
     491    // the page calls preventDefault() in a drag event but doesn't set dropEffect.
     492    if (srcOpMask == DragOperationEvery)
     493        return DragOperationCopy;
     494    if (srcOpMask == DragOperationNone)
     495        return DragOperationNone;
     496    if (srcOpMask & DragOperationMove || srcOpMask & DragOperationGeneric)
     497        return DragOperationMove;
     498    if (srcOpMask & DragOperationCopy)
     499        return DragOperationCopy;
     500    if (srcOpMask & DragOperationLink)
     501        return DragOperationLink;
     502   
     503    // FIXME: Does IE really return "generic" even if no operations were allowed by the source?
     504    return DragOperationGeneric;
     505}
     506
    488507bool DragController::tryDHTMLDrag(DragData* dragData, DragOperation& operation)
    489508{
    … …  
    507526
    508527    operation = clipboard->destinationOperation();
    509     if (!(srcOpMask & operation)) {
     528    if (clipboard->dropEffectIsUninitialized())
     529        operation = defaultOperationForDrag(srcOpMask);
     530    else if (!(srcOpMask & operation)) {
    510531        // The element picked an operation which is not supported by the source
    511532        operation = DragOperationNone;
Note: See TracChangeset for help on using the changeset viewer.