[Webkit-unassigned] [Bug 35314] smartdelete should only occur after double-click

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=35314


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #50147|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #7 from David Levin <levin at chromium.org>  2010-03-11 17:08:13 PST ---
(From update of attachment 50147)
I have a few comments but they are all about pretty small stuff, so feel free
to fix these on landing (unless you want to me to look it over again).


This current patch isn't really about the bug that it is attached to. It is
doing some different (which is a good step to fix the bug). 

I wish there was a separate bug just because I find it confusing to read the
ChangeLog which says "smartdelete should only occur after double-click" but
this patch doesn't fix that.

> diff --git a/LayoutTests/editing/deleting/non-smart-delete.html b/LayoutTests/editing/deleting/non-smart-delete.html

> +<br>
> +The first word should be deleted. The space following it should remain. It should like this this (currently this is broken and the space is deleted):

"It should like this this"
I think you meant "It should look like this"

If something is broken, it would be nice to reference a bug number. (I think it
is this bug.)


> diff --git a/LayoutTests/editing/deleting/smart-delete-003.html b/LayoutTests/editing/deleting/smart-delete-003.html
>  if (window.layoutTestController)
>       layoutTestController.dumpEditingCallbacks();
>  </script>
> +<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
>  <p>This tests deleting a selection created with a word granularity.  To run it manually, double click on 'bar' and hit delete.  You should see 'foo bar'.</p>

Shouldn't you see 'foo baz'?

> diff --git a/LayoutTests/editing/deleting/smart-delete-004.html b/LayoutTests/editing/deleting/smart-delete-004.html
>  <p>This tests deleting a selection created with a word granularity.  To run it manually, double click on 'bar' and hit forward delete.  You should see 'foo bar'.</p>

'foo baz' here as well.


> diff --git a/LayoutTests/editing/pasteboard/drag-drop-modifies-page.html b/LayoutTests/editing/pasteboard/drag-drop-modifies-page.html
> +<p>This tests non-smartmove drag/drop. The space should be deleted on move,
> +but not reinserted on drop, resulting in "worldhello". Currently there's a bug
> +where the space is reinserted on drop.</p>

Bug # reference would be nice.


> diff --git a/LayoutTests/editing/pasteboard/smart-drag-drop.html b/LayoutTests/editing/pasteboard/smart-drag-drop.html
> +function editingTest() {
> +  
> +    if (!window.eventSender)
> +        return;
> +    doubleClickAtSelectionStart();
> +
> +    // Drag 'hello'
> +    var e = document.getElementById("dragme");
> +    x = e.offsetLeft + 10;

Why 10 instead of something based on offsetWidth?

> +    // and drop it off to the right somewhere

Would be nice to add a "." at the end.


> +<p>Tests that drag/drop after double-click does a smart drag. Specifically the end result should have a space: "world hello".</p>

This should include instructions for how to do the test manually as well.



> diff --git a/LayoutTests/editing/selection/script-tests/delete-word-granularity-text-control.js b/LayoutTests/editing/selection/script-tests/delete-word-granularity-text-control.js
> +description("Test that setSelectedRange resets the seleciton granularity to CharacterGranularity.");

Add info about how to do it manually.

> +var successfullyParsed = true;
> \ No newline at end of file

Please add the newline.


> diff --git a/LayoutTests/editing/style/style-boundary-005.html b/LayoutTests/editing/style/style-boundary-005.html
> @@ -51,7 +51,8 @@ Pasting at style boundary does not crash or produce empty style span(s).
>  <div class="expected-results">
>  Expected Results:
>  <br>
> -Should see this content in the red box below: <br><div>one two three <b>four</b> one</div>
> +Should see this content in the red box below (currently there's a bug where
> +there is an extra space after the bold element: <br><div>one two three <b>four</b>one</div>

Please close your parenthetical remark which begins "(currently"
Also bug reference?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.