Changeset 56567 in webkit


Ignore:
Timestamp:
Mar 25, 2010 1:21:45 PM (14 years ago)
Author:
ojan@chromium.org
Message:

2010-03-25 Ojan Vafai <​ojan@chromium.org>

Reviewed by David Levin.

mouse-based selections are always directional on Window/Linux
​https://bugs.webkit.org/show_bug.cgi?id=25195

The tests are modified to expect different things for Win/Linux vs. Mac.

  • editing/selection/5195166-1.html:
  • editing/selection/extend-after-mouse-selection.html:
  • editing/selection/extend-selection-after-double-click-expected.txt:
  • editing/selection/extend-selection-after-double-click.html:
  • platform/chromium-linux/editing/selection/5195166-1-expected.checksum: Removed.
  • platform/chromium-linux/editing/selection/5195166-1-expected.png: Removed.
  • platform/chromium-win/editing/selection/5195166-1-expected.checksum: Removed.
  • platform/chromium-win/editing/selection/5195166-1-expected.png: Removed.
  • platform/chromium-win/editing/selection/5195166-1-expected.txt: Removed.
  • platform/mac/editing/selection/5195166-1-expected.checksum: Removed.
  • platform/mac/editing/selection/5195166-1-expected.png: Removed.
  • platform/mac/editing/selection/5195166-1-expected.txt:
  • platform/qt/editing/selection/5195166-1-expected.txt: Removed.
  • platform/win/editing/selection/extend-after-mouse-selection-expected.txt:

2010-03-25 Ojan Vafai <​ojan@chromium.org>

Reviewed by David Levin.

mouse-based selections are always directional on Windows/Linux
​https://bugs.webkit.org/show_bug.cgi?id=25195

Change m_lastChangeWasHorizontalExtension to m_isDirectional
and make m_isDirectional always be true for Windows/Linux.

  • editing/SelectionController.cpp: (WebCore::SelectionController::SelectionController): (WebCore::SelectionController::setSelection): (WebCore::SelectionController::setIsDirectional): (WebCore::SelectionController::willBeModified): When double-clicking, the base/extent will be in the middle of the selection instead of the start/end of it. Changed to maintain that modifications after double-click still move the start/end of the selection, not the base/extent. (WebCore::SelectionController::modify):
  • editing/SelectionController.h:
  • page/EventHandler.cpp: (WebCore::EventHandler::handleMousePressEventSingleClick): (WebCore::EventHandler::updateSelectionForMouseDrag):
Location:
trunk
Files:
8 deleted
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r56562 r56567  
     12010-03-25  Ojan Vafai  <ojan@chromium.org>
     2
     3        Reviewed by David Levin.
     4
     5        mouse-based selections are always directional on Window/Linux
     6        https://bugs.webkit.org/show_bug.cgi?id=25195
     7
     8        The tests are modified to expect different things for Win/Linux vs. Mac.
     9
     10        * editing/selection/5195166-1.html:
     11        * editing/selection/extend-after-mouse-selection.html:
     12        * editing/selection/extend-selection-after-double-click-expected.txt:
     13        * editing/selection/extend-selection-after-double-click.html:
     14        * platform/chromium-linux/editing/selection/5195166-1-expected.checksum: Removed.
     15        * platform/chromium-linux/editing/selection/5195166-1-expected.png: Removed.
     16        * platform/chromium-win/editing/selection/5195166-1-expected.checksum: Removed.
     17        * platform/chromium-win/editing/selection/5195166-1-expected.png: Removed.
     18        * platform/chromium-win/editing/selection/5195166-1-expected.txt: Removed.
     19        * platform/mac/editing/selection/5195166-1-expected.checksum: Removed.
     20        * platform/mac/editing/selection/5195166-1-expected.png: Removed.
     21        * platform/mac/editing/selection/5195166-1-expected.txt:
     22        * platform/qt/editing/selection/5195166-1-expected.txt: Removed.
     23        * platform/win/editing/selection/extend-after-mouse-selection-expected.txt:
     24
    1252010-03-25  Simon Fraser  <simon.fraser@apple.com>
    226

  • trunk/LayoutTests/editing/selection/5195166-1.html

    r29149 r56567  
    11<p>This tests for a bug where extending a selection created with the mouse would blow it away before extending it.</p>
    2 <div id="div" contenteditable="true">There should be five characters selected in this sentence.</div>
     2<div id="div" contenteditable="true">There should be six characters selected in this sentence on Mac and four characters selected on Win/Linux.</div>
    33<ul id="console"></ul>
    44<script>
    … …  
    1111}
    1212if (window.layoutTestController) {
     13    layoutTestController.dumpAsText()
     14
    1315    var text = document.getElementById("div").firstChild;
    1416    var selection = window.getSelection();
    … …  
    2022   
    2123    selection.setBaseAndExtent(text, 3 + 5, text, 3);
    22     // Extending this 5 character selection will select 6 characters.
     24    // Extending this 5 character selection will select 6 characters on mac,
     25    // but shrink the selection on win/linux.
    2326    layoutTestController.execCommand("MoveForwardAndModifySelection");
    24     // Extending it in this way flips the base and the extent.
    25     if (selection.extentOffset - selection.baseOffset != 6)
    26         log("Failure: Selection isn't the right size.");
     27    // Extending it in this way flips the anchor and the focus on Mac.
     28    var onMacPlatform = navigator.userAgent.search(/\bMac OS X\b/) != -1;
     29    if (onMacPlatform && selection.focusOffset - selection.anchorOffset == 6 ||
     30        !onMacPlatform && selection.anchorOffset - selection.focusOffset == 4)
     31        log("Success");
    2732    else
    28         log ("Success");
     33        log("Failure: Selection isn't the right size. anchorOffset=" + selection.anchorOffset +
     34            " focusOffset=" + selection.focusOffset + " onMacPlatform: " + onMacPlatform);
    2935} else
    3036    log ("Failure: This test cannot be run manually.")

  • trunk/LayoutTests/editing/selection/extend-after-mouse-selection.html

    r54980 r56567  
    1 <html> 
     1<html>
    22<head>
    33
    44<style>
    5 .editing { 
    6     border: 2px solid red; 
    7     font-size: 24px; 
     5.editing {
     6    border: 2px solid red;
     7    font-size: 24px;
    88}
    99</style>
    … …  
    2525    // character granularity (i.e. that it just doesn't work).
    2626    eventSender.mouseDown();
    27     eventSender.mouseUp();   
     27    eventSender.mouseUp();
    2828    eventSender.mouseDown();
    2929    eventSender.mouseMoveTo(endTarget.offsetLeft, endTarget.offsetTop + 10);
    30     eventSender.mouseUp();   
     30    eventSender.mouseUp();
    3131
    3232    assertSelectionAt(startTarget.firstChild, 0, endTarget.firstChild, 2);
    … …  
    3434    extendSelectionBackwardByCharacterCommand();
    3535
    36     // The first character-granularity selection after a mouse-selection resets the anchor/focus.
    37     // FIXME: On Win/Linux the anchor/focus should be fixed after the mouse-selection.
    38     assertSelectionAt(endTarget.firstChild, 2, startTarget.previousSibling, 1);
     36
     37    // On Win/Linux the anchor is be fixed after the mouse-selection and never changes.
     38    // On Mac, the first character-granularity selection after a mouse-selection resets the anchor/focus.
     39    if (onMacPlatform)
     40        assertSelectionAt(endTarget.firstChild, 2, startTarget.previousSibling, 1);
     41    else
     42        assertSelectionAt(startTarget.firstChild, 0, endTarget.firstChild, 1);
    3943
    4044    extendSelectionForwardByCharacterCommand();
    41     assertSelectionAt(endTarget.firstChild, 2, startTarget.firstChild, 0);
    42    
     45    if (onMacPlatform)
     46        assertSelectionAt(endTarget.firstChild, 2, startTarget.firstChild, 0);
     47    else
     48        assertSelectionAt(startTarget.firstChild, 0, endTarget.firstChild, 2);
     49
    4350    extendSelectionForwardByLineBoundaryCommand();
    44    
     51
    4552    if (onMacPlatform) {
    4653        // FIXME: This encodes what TextEdit does. Currently WebKit does the wrong thing.
    4754        // The selection should go from the beginning of startTarget to the end of the line that endTarget is on.
    4855        assertSelectionAt(endTarget.nextSibling, 1, startTarget.firstChild, 0);
    49     } else {
    50         // Seleciton goes from the end of "ef" to the first BR element in the contentEditable region.
    51         assertSelectionAt(endTarget.firstChild, 2, document.getElementById('test'), 3);
    52     }
    53    
     56    } else
     57        assertSelectionAt(startTarget.firstChild, 0, endTarget.nextSibling, 1);
     58
    5459    extendSelectionBackwardByLineBoundaryCommand();
    5560
    … …  
    5964        assertSelectionAt(endTarget.nextSibling, 1, startTarget.previousSibling, 0);
    6065    } else
    61         assertSelectionAt(endTarget.firstChild, 2, startTarget.previousSibling, 0);
     66        assertSelectionAt(startTarget.firstChild, 0, document.getElementById('test'), 4);
    6267}
    6368
    6469</script>
    6570
    66 <title>Editing Test</title> 
    67 </head> 
     71<title>Editing Test</title>
     72</head>
    6873<body>
    6974<div contenteditable id="root" class="editing">

  • trunk/LayoutTests/editing/selection/extend-selection-after-double-click-expected.txt

    r45945 r56567  
    11This tests modifying a selection created with a double click with shift arrow key.
    22
    3 This test does not run interactively. It uses the event sender to do mouse clicks. To run it manually, double click on the blue "g", then press shift-left-arrow twice. Then repeat those steps again. The selection should include the words "a paragraph".
     3This test does not run interactively. It uses the event sender to do mouse clicks. To run it manually, double click on the blue "g", then press shift-left-arrow twice. Then repeat those steps again. The selection should include the words "a paragraph" on mac and "paragra" on win/linux..
    44
    55This is a paragraph.
    66
    7 SUCCESS: The selection was extended to the left of the word.
     7SUCCESS

  • trunk/LayoutTests/editing/selection/extend-selection-after-double-click.html

    r45945 r56567  
    3535        var result;
    3636        var selectedText = getSelection().toString();
    37         if (selectedText == "a paragraph")
    38             result = "SUCCESS: The selection was extended to the left of the word.";
     37        var onMacPlatform = navigator.userAgent.search(/\bMac OS X\b/) != -1;
     38        var expectedText = onMacPlatform ? "a paragraph" : "paragra";
     39        if (selectedText == expectedText)
     40            result = "SUCCESS";
    3941        else
    40             result = "FAILURE: The selected text is \"" + selectedText + "\".";
     42            result = "FAILURE: The selected text is \"" + selectedText + "\" and should be \"" + expectedText + "\".";
    4143        document.getElementById("result").firstChild.data = result;
    4244    }
    … …  
    5052    To run it manually, double click on the blue "g", then press shift-left-arrow twice.
    5153    Then repeat those steps again.
    52     The selection should include the words "a paragraph".
     54    The selection should include the words "a paragraph" on mac and "paragra" on win/linux..
    5355</p>
    5456<p>This is a para<span style="color:blue" id="start">g</span>raph.</p>

  • trunk/LayoutTests/platform/mac/editing/selection/5195166-1-expected.txt

    r43215 r56567  
    1 layer at (0,0) size 800x600
    2   RenderView at (0,0) size 800x600
    3 layer at (0,0) size 800x600
    4   RenderBlock {HTML} at (0,0) size 800x600
    5     RenderBody {BODY} at (8,8) size 784x576
    6       RenderBlock {P} at (0,0) size 784x18
    7         RenderText {#text} at (0,0) size 704x18
    8           text run at (0,0) width 704: "This tests for a bug where extending a selection created with the mouse would blow it away before extending it."
    9       RenderBlock {DIV} at (0,34) size 784x18
    10         RenderText {#text} at (0,0) size 355x18
    11           text run at (0,0) width 355: "There should be five characters selected in this sentence."
    12       RenderBlock {UL} at (0,68) size 784x18
    13         RenderListItem {LI} at (40,0) size 744x18
    14           RenderListMarker at (-17,0) size 7x18: bullet
    15           RenderText {#text} at (0,0) size 50x18
    16             text run at (0,0) width 50: "Success"
    17 selection start: position 3 of child 0 {#text} of child 2 {DIV} of child 1 {BODY} of child 0 {HTML} of document
    18 selection end:   position 9 of child 0 {#text} of child 2 {DIV} of child 1 {BODY} of child 0 {HTML} of document
     1This tests for a bug where extending a selection created with the mouse would blow it away before extending it.
     2
     3There should be six characters selected in this sentence on Mac and four characters selected on Win/Linux.
     4Success

  • trunk/LayoutTests/platform/win/editing/selection/extend-after-mouse-selection-expected.txt

    r54980 r56567  
    33ghi
    44PASS Selection is [anchorNode: [object Text](bc) anchorOffset: 0 focusNode: [object Text](ef) focusOffset: 2 isCollapsed: false]
    5 PASS Selection is [anchorNode: [object Text](ef) anchorOffset: 2 focusNode: [object Text](a ) focusOffset: 1 isCollapsed: false]
    6 PASS Selection is [anchorNode: [object Text](ef) anchorOffset: 2 focusNode: [object Text](bc) focusOffset: 0 isCollapsed: false]
    7 PASS Selection is [anchorNode: [object Text](ef) anchorOffset: 2 focusNode: [object HTMLElement](null) focusOffset: 3 isCollapsed: false]
    8 PASS Selection is [anchorNode: [object Text](ef) anchorOffset: 2 focusNode: [object Text](a ) focusOffset: 0 isCollapsed: false]
     5PASS Selection is [anchorNode: [object Text](bc) anchorOffset: 0 focusNode: [object Text](ef) focusOffset: 1 isCollapsed: false]
     6PASS Selection is [anchorNode: [object Text](bc) anchorOffset: 0 focusNode: [object Text](ef) focusOffset: 2 isCollapsed: false]
     7PASS Selection is [anchorNode: [object Text](bc) anchorOffset: 0 focusNode: [object Text]( ) focusOffset: 1 isCollapsed: false]
     8PASS Selection is [anchorNode: [object Text](bc) anchorOffset: 0 focusNode: [object HTMLElement](null) focusOffset: 4 isCollapsed: false]
    99PASS successfullyParsed is true
    1010

  • trunk/WebCore/ChangeLog

    r56565 r56567  
     12010-03-25  Ojan Vafai  <ojan@chromium.org>
     2
     3        Reviewed by David Levin.
     4
     5        mouse-based selections are always directional on Windows/Linux
     6        https://bugs.webkit.org/show_bug.cgi?id=25195
     7
     8        Change m_lastChangeWasHorizontalExtension to m_isDirectional
     9        and make m_isDirectional always be true for Windows/Linux.
     10
     11        * editing/SelectionController.cpp:
     12        (WebCore::SelectionController::SelectionController):
     13        (WebCore::SelectionController::setSelection):
     14        (WebCore::SelectionController::setIsDirectional):
     15        (WebCore::SelectionController::willBeModified):
     16        When double-clicking, the base/extent will be in the middle
     17        of the selection instead of the start/end of it. Changed to
     18        maintain that modifications after double-click still move the
     19        start/end of the selection, not the base/extent.
     20        (WebCore::SelectionController::modify):
     21        * editing/SelectionController.h:
     22        * page/EventHandler.cpp:
     23        (WebCore::EventHandler::handleMousePressEventSingleClick):
     24        (WebCore::EventHandler::updateSelectionForMouseDrag):
     25
    1262010-03-25  Simon Fraser  <simon.fraser@apple.com>
    227

  • trunk/WebCore/editing/SelectionController.cpp

    r56175 r56567  
    7171    , m_needsLayout(true)
    7272    , m_absCaretBoundsDirty(true)
    73     , m_lastChangeWasHorizontalExtension(false)
    7473    , m_isDragCaretController(isDragCaretController)
    7574    , m_isCaretBlinkingSuspended(false)
    … …  
    7877    , m_caretPaint(true)
    7978{
     79    setIsDirectional(false);
    8080}
    8181
    … …  
    110110    m_granularity = granularity;
    111111
    112     m_lastChangeWasHorizontalExtension = false;
     112    setIsDirectional(false);
    113113
    114114    if (m_isDragCaretController) {
    … …  
    233233        setSelection(VisibleSelection(), false, false);
    234234}
     235   
     236void SelectionController::setIsDirectional(bool isDirectional)
     237{
     238    Settings* settings = m_frame ? m_frame->settings() : 0;
     239    m_isDirectional = !settings || (settings->editingBehavior() == EditingMacBehavior && isDirectional);
     240}
    235241
    236242void SelectionController::willBeModified(EAlteration alter, EDirection direction)
    237243{
    238244    if (alter != EXTEND)
    239         return;
    240     if (m_lastChangeWasHorizontalExtension)
    241245        return;
    242246
    243247    Position start = m_selection.start();
    244248    Position end = m_selection.end();
    245     // FIXME: This is probably not correct for right and left when the direction is RTL.
    246     switch (direction) {
     249
     250    if (m_isDirectional) {
     251        // Make base and extent match start and end so we extend the user-visible selection.
     252        // This only matters for cases where base and extend point to different positions than
     253        // start and end (e.g. after a double-click to select a word).
     254        if (m_selection.isBaseFirst()) {
     255            m_selection.setBase(start);
     256            m_selection.setExtent(end);           
     257        } else {
     258            m_selection.setBase(end);
     259            m_selection.setExtent(start);
     260        }
     261    } else {
     262        // FIXME: This is probably not correct for right and left when the direction is RTL.
     263        switch (direction) {
    247264        case RIGHT:
    248265        case FORWARD:
    … …  
    255272            m_selection.setExtent(start);
    256273            break;
     274        }
    257275    }
    258276}
    … …  
    598616        SelectionController trialSelectionController;
    599617        trialSelectionController.setSelection(m_selection);
    600         trialSelectionController.setLastChangeWasHorizontalExtension(m_lastChangeWasHorizontalExtension);
     618        trialSelectionController.setIsDirectional(m_isDirectional);
    601619        trialSelectionController.modify(alter, dir, granularity, false);
    602620
    … …  
    663681    setNeedsLayout();
    664682
    665     m_lastChangeWasHorizontalExtension = alter == EXTEND;
     683    setIsDirectional(alter == EXTEND);
    666684
    667685    return true;
    … …  
    686704        SelectionController trialSelectionController;
    687705        trialSelectionController.setSelection(m_selection);
    688         trialSelectionController.setLastChangeWasHorizontalExtension(m_lastChangeWasHorizontalExtension);
     706        trialSelectionController.setIsDirectional(m_isDirectional);
    689707        trialSelectionController.modify(alter, verticalDistance, false);
    690708
    … …  
    756774        m_granularity = CharacterGranularity;
    757775
    758     m_lastChangeWasHorizontalExtension = alter == EXTEND;
     776    setIsDirectional(alter == EXTEND);
    759777
    760778    return true;

  • trunk/WebCore/editing/SelectionController.h

    r56175 r56567  
    9898    void setNeedsLayout(bool flag = true);
    9999
    100     void setLastChangeWasHorizontalExtension(bool b) { m_lastChangeWasHorizontalExtension = b; }
     100    void setIsDirectional(bool);
    101101    void willBeModified(EAlteration, EDirection);
    102102   
    … …  
    190190    bool m_needsLayout; // true if m_caretRect and m_absCaretBounds need to be calculated
    191191    bool m_absCaretBoundsDirty;
    192     bool m_lastChangeWasHorizontalExtension;
     192    bool m_isDirectional;
    193193    bool m_isDragCaretController;
    194194    bool m_isCaretBlinkingSuspended;

  • trunk/WebCore/page/EventHandler.cpp

    r56327 r56567  
    357357
    358358    if (extendSelection && newSelection.isCaretOrRange()) {
    359         m_frame->selection()->setLastChangeWasHorizontalExtension(false);
     359        m_frame->selection()->setIsDirectional(false);
    360360       
    361361        // See <rdar://problem/3668157> REGRESSION (Mail): shift-click deselects when selection
    … …  
    592592
    593593    if (m_frame->shouldChangeSelection(newSelection)) {
    594         m_frame->selection()->setLastChangeWasHorizontalExtension(false);
     594        m_frame->selection()->setIsDirectional(false);
    595595        m_frame->selection()->setSelection(newSelection, m_frame->selectionGranularity());
    596596    }
Note: See TracChangeset for help on using the changeset viewer.