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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Mar 17 18:30:29 PDT 2010


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


David Levin <levin at chromium.org> changed:

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




--- Comment #17 from David Levin <levin at chromium.org>  2010-03-17 18:30:29 PST ---
(From update of attachment 50616)
Just a few minor things, which you can fix when landing it.



> diff --git a/WebCore/editing/VisibleSelection.h b/WebCore/editing/VisibleSelection.h
> +    void validate(TextGranularity granularity = CharacterGranularity);

Pls remove param name "granularity" as it adds no information here.

> +    void setStartAndEndFromBaseAndExtentRespectingGranularity(TextGranularity granularity);

Pls remove param name "granularity" as it adds no information here.


> diff --git a/WebCore/page/DragController.cpp b/WebCore/page/DragController.cpp
> +            // NSTextView behavior is to always smartdelete on moving a selection,

Rather than lower casing smartdelete, smartinsert, selectiongranularity,
wordgranularity (which I find hard to read), please consider either
UpperCasing both words or separating them or formatting as is done in the code
smartDelete.

Personally, I prefer smart delete, smart insert, selection granularity, word
granularity.

> +            // but only to smartinsert if the selectiongranularity is wordgranularity.



> diff --git a/WebKit/chromium/ChangeLog b/WebKit/chromium/ChangeLog
> +2010-03-12  Ojan Vafai  <ojan at chromium.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Need a short description and bug URL (OOPS!)

oops indeed :)

> +
> +        * src/WebFrameImpl.cpp:
> +        (WebKit::WebFrameImpl::selectWordAroundPosition):
> +

> diff --git a/WebKit/chromium/src/WebFrameImpl.cpp b/WebKit/chromium/src/WebFrameImpl.cpp

>      if (frame->shouldChangeSelection(selection))
As you mentioned, this should be:

      if (frame->shouldChangeSelection(selection)) {
          TextGranularity granularity = CharacterGranularity;
          if (selection.isRange())
              granularity = WordGranularity;
          frame->selection()->setSelection(selection, granularity);
      }

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



More information about the webkit-unassigned mailing list