[Webkit-unassigned] [Bug 28696] Middle click doesn't trigger onpaste event in Chromium

[bugzilla-daemon at webkit.org]

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


Darin Fisher (:fishd, Google) <fishd at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #38938|review+                     |review-
               Flag|                            |




--- Comment #11 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2009-09-02 14:59:49 PDT ---
(From update of attachment 38938)
> Index: WebCore/platform/chromium/ChromiumBridge.h
...
> +        static String clipboardReadPlainText(PasteboardPrivate::TargetClipboard);
>          static void clipboardReadHTML(String*, KURL*);

So, I guess it is implied that the target is the system clipboard for all
of the methods that do not have a TargetClipboard parameter?  It might be
good to write a comment saying that.


> Index: WebCore/platform/chromium/ClipboardChromium.cpp
...
>  #include "HTMLNames.h"
>  #include "NamedAttrMap.h"
>  #include "MIMETypeRegistry.h"
> +#include "Pasteboard.h"
>  #include "markup.h"
>  #include "NamedNodeMap.h"
>  #include "PlatformString.h"

nit: includes should be in alphabetical order


> Index: WebCore/platform/chromium/PasteboardChromium.cpp
...
> +bool Pasteboard::isSelectionMode() const
> +{
> +      return m_selectionMode;

nit: indentation should be 4 white spaces


> +void Pasteboard::setSelectionMode(bool selectionMode)
> +{
> +      m_selectionMode = selectionMode;

ditto


>  PassRefPtr<DocumentFragment> Pasteboard::documentFragment(Frame* frame, PassRefPtr<Range> context, bool allowPlainText, bool& chosePlainText)
>  {
>      chosePlainText = false;
> +    PasteboardPrivate::TargetClipboard target = m_selectionMode ?  PasteboardPrivate::PrimarySelection : PasteboardPrivate::Clipboard;

nit: only one space after "?"


> Index: WebCore/platform/chromium/PasteboardPrivate.h
...
> +        enum TargetClipboard {
> +            Clipboard,
> +            PrimarySelection,
> +        };

Please change TargetClipboard to ClipboardTarget to match the naming
convention already in use (like ClipboardFormat).

I would also change the enum values to something like the following:

  SystemTarget
  SelectionTarget

Where SystemTarget refers to the system clipboard and SelectionTarget
refers to the selection clipboard.

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