clipboard.autocopy does not work on Windows

NEW
Unassigned

Status

()

defect
--
minor
17 years ago
a year ago

People

(Reporter: mofo_the_best15, Unassigned)

Tracking

Trunk
x86
Windows XP
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 obsolete attachments)

(Reporter)

Description

17 years ago
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.1b) Gecko/20020721
BuildID:    2002072104

This is from http://www.mozilla.org/unix/customizing.html
"// Uncomment this to turn off Unix-style autocopy
// (or set it to true to turn it on on non-Unix platforms):
//user_pref("clipboard.autocopy", false);"


Reproducible: Always
Steps to Reproduce:
1. add user_pref("clipboard.autocopy", false); to user.js
2. exit mozilla (quick launch too )
3. ummmmm open mozilla
4. highlight text and paste

Actual Results:  highlighted text is not copied to clipboard  

Expected Results:  text is copied to clipboard 

i know it is one of the hidden prefs but i'd to see it working in Windowz ( or
maybe it is just XP; sorry no access to other Winz w/ Mozilla ).
(Reporter)

Comment 1

17 years ago
--> 1. add user_pref("clipboard.autocopy", false); to user.js <--
it should be TRUE not false ( i just c&p from top :)

Comment 2

17 years ago
I found that pref too a while back, in all.js, and caught hope - but it doesn't
work on WinXP here either. COnfirming.

No idea if autocopy even is supposed to work on Windows. Default is "false". The
switch may be *nix specific. Would be great to have on Windows of course.

Over to selection for comment.
Assignee: Matti → mjudge
Status: UNCONFIRMED → NEW
Component: Browser-General → Selection
Ever confirmed: true
QA Contact: asa → pmac

Comment 3

17 years ago
It doesn't work under Windows 2000 either (1.1 final). Too bad, it's a nice idea.

Comment 4

17 years ago
I have confirmed this to be a bug on Mozilla 1.2.1 on Windows XP as well.

Comment 5

16 years ago
Doesn't work on Windows 98 either.  Mozilla 1.3b.  Used about:config to make the
change, confirmed that prefs.js was changed properly, but after restart Mozilla
didn't autocopy to the clipboard.

Shame, I'm in the process of a large copy/paste job which can't be scripted;
this option would have saved me considerable time.

Comment 6

16 years ago
Verified that it also does not work on NT4.0 SP6.0a using mozilla 1.3b

Comment 7

16 years ago
I'm having related problems (at least I think they're related) using 1.3 under
Linux.

With clipboard.autocopy set to false, /all/ copy-to-clipboard operations fail,
even right-click-on-selected-text and choosing "Copy".  Nothing from the browser
ever makes it into the clipboard.

Leaving it true causes a really, really annoying fight between mozilla and any
tool which scans clipboard text for special actions (e.g., KDE's klipper).
Simply selecting the URL text in the urlbar -- even without actually
right-clicking and choosing "copy" -- will copy the URL into the clipboard.
Which then triggers a popup or some other message from the special-actions
tool.  I was really expecting that the autocopy would only effect text in the
normal browser window, not the urlbar.

Comment 8

16 years ago
This bug also exists in
Windows 2000 5.00.2195 Service Pack 3
Mozilla 1.4 RC1

I have the Optimoz mouse gestures plugin installed that may cause some conflict.

Comment 9

16 years ago
Hmmm... I had the radial context / mouse gestures plugin from mozdev installed
at the time I wrote comment 7.  Since then I've had to reinstall mozilla for
unrelated reasons.  I've never gotten around to reinstalling that (very useful)
plugin, and I no longer experience the fights over the clipboard text.

Comment 10

16 years ago
This is a bug in W2K SP4 in Thunderbird 0.1 Milestone as well
*** Bug 219966 has been marked as a duplicate of this bug. ***

Comment 12

15 years ago
I wrote comment #7 and then comment #9.  Using 1.5, also with no weird plugins,
behavior remained good.  Under 1.6, the applications are fighting over the
clipboard again.

Setting this to false makes them not fight, but still kills all clipboard
copying, everywhere, even manual Edit -> Copy and right-click -> Copy.  :-(

Comment 13

15 years ago
Please change the summary to the fact that all Windows versions are affected.

"clipboard.autocopy does not work on Windows"

Comment 14

15 years ago
Does not work with XP SP1 and Mozilla 1.7b. If this was fixed I think my surfing
life would be complete.
*** Bug 258860 has been marked as a duplicate of this bug. ***

Comment 16

14 years ago
Everyone who works on unix as well as on windows will appreciate a fix.
Will there ever be a Windows fix for this? 
Or is it that this will never work on Win32 and is just for *nix? Just knowing either way would be helpful...

Reporter/someone with Summary change access needs to change this to Windows (All)

Updated

13 years ago
Assignee: mjudge → selection
QA Contact: pmac
Summary: clipboard.autocopy does not work ( i'm on XP ) → clipboard.autocopy does not work on Windows

Comment 18

13 years ago
If this worked, I'd be able to test some "Linux-only" bugs on Windows.

Comment 19

13 years ago
bryner, it doesn't seem obvious to me why this doesn't work on windows.

Comment 20

12 years ago
clipboard.autocopy also doesn't seem to work on Mac OS X. I really miss this feature, on unix machines I use it every day, and I always have to remember it doesn't work on my mac... :(

Comment 21

12 years ago
(In reply to comment #20)
> clipboard.autocopy also doesn't seem to work on Mac OS X.
There is a seperate bug filed for this on OS X: Bug 392161.

Related bug 265868, will probably occur on Windows as well if this is fixed. (when this pref is enabled)
Assignee: selection → nobody
QA Contact: selection

Comment 22

9 years ago
this is STILL not working in 3.6?

Comment 23

6 years ago
still not working on xp

Comment 24

6 years ago
Just another person hoping this feature will be enabled on Windows. 

Maybe they don't want people messing about with about:config, or adding another checkbox to the settings dialog. 

It's a pity though - this would be really useful to a lot of people who aren't aware such a thing exists. 
Especially considering the amount of copying and pasting people do from websites. Well, I do a lot. 

It could be another selling point against IE and Chrome.

Comment 25

5 years ago
This patch will enable auto copy on all platforms.

Comment 26

5 years ago
can some1 review my patch?
please

Comment 27

5 years ago
the problem is also that if u enable clipboard.autocopy pref on windows (and leave it on), then it will add useless selection listener (it creates memory leaks)

maybe if u dont want enable this feature in windows then hide this pref?

Comment 28

5 years ago
the problem because why its not working is there is no selection clipboard in windows
there is only global clipboard

Updated

5 years ago
Attachment #8434121 - Flags: review?(hskupin)
Comment on attachment 8434121 [details] [diff] [review]
enable-auto-copy-on-all-platforms.patch

Review of attachment 8434121 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for the patch! But I'm not a reviewer for this component. Maybe Masayuki can help here.
Attachment #8434121 - Flags: review?(hskupin) → review?(masayuki)
Comment on attachment 8434121 [details] [diff] [review]
enable-auto-copy-on-all-platforms.patch

This patch may destroy global clipbord data unexpectedly. I guess that some users who don't understand the pref well have already changed the pref true. Then, such users would file INVALID or WONTFIX bugs.

I'd like to suggest that we should add new bool pref like "clipboard.autocopy.allow_to_use_global_clipboard".

Then, this patch can be safer and simpler.

First, you should add two static members to nsAutoCopyListener. One is |bool sSelectionClipboardSupported|. It must not be changed dynamically if the platform changes to support selection clipboard. Therefore, the result must be able to be stored as a simple bool pref. You should cache the result of nsIClipboard::SupportsSelectionClipboard() at the constructor of nsAutoCopy. I.e., you need to move the constructor implementation from nsSelection.h to nsAutoCopyListener.h to nsSelection.cpp.

Next, the other is |bool sUseGlobalClipboard|. This should be used by Preferences::AddBoolVarCache() (http://mxr.mozilla.org/mozilla-central/source/modules/libpref/public/Preferences.h#270) and it should store the new pref's value. You should initialize this in the constructor too.

Then, nsAutoCopyListener::NotifySelectionChanged() would be like this:

> if (sUseGlobalClipboard) {
>   return nsCopySupport::HTMLCopy(... nsIClipboard::kGlobalClipboard);
> }
> 
> if (!sSelectionClipboardSupported) {
>   return NS_OK;
> }
> 
> return nsCopySupport::HTMLCopy(... nsIClipboard::kSelectionClipboard);

I.e., the new pref allows Linux and UNIX users to use same behavior as the other platforms.

I'd like to ask bz about this idea. How do you think, bz? (IIRC, bz commented around autocopy.)
Attachment #8434121 - Flags: feedback?(bzbarsky)

Comment 31

5 years ago
Or maybe make auto copy intelligent/smart?

Look at my addon:
https://addons.mozilla.org/de/firefox/addon/quick-copy-2/

This does only copy text if you hold down the mouse for 0.8 secs.
When its copied, the selection will be collapsed.
So, the user can easily see that the text is copied.

Comment 32

5 years ago
but
I like your idea also :)

Comment 33

5 years ago
Here is my code from Quick Copy 2 addon:

const selectionListener = {
  copyTimer: null,
  notifySelectionChanged: function(doc, sel, reason) {
  
    if (reason & Ci.nsISelectionListener.MOUSEDOWN_REASON) {
      if (sel.isCollapsed)
        return;
        
      if (!/\S+/.test(sel.toString()))
        return; 
  
      this._win = doc.defaultView;
      
      this.cancelCopyTimer();
      this.copyTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
      this.copyTimer.initWithCallback(this, 800,
                                      Ci.nsITimer.TYPE_ONE_SHOT);
    } 
    else if (reason == Ci.nsISelectionListener.MOUSEUP_REASON) {
      this.cancelCopyTimer();
    }
  },
  
  cancelCopyTimer: function() {
    if (this.copyTimer) {
      this.copyTimer.cancel();
      this.copyTimer = null;
    }
  },
  
  notify: function(aTimer) {
    if (aTimer == this.copyTimer) {
      this.copyTimer = null;
      let sel = this._win.getSelection();
      if (!sel) return;
      let toCopy = sel.toString().trim();
      if (toCopy) {
        let clipboardHelper = Cc["@mozilla.org/widget/clipboardhelper;1"].
                              getService(Ci.nsIClipboardHelper);
        clipboardHelper.copyString(toCopy, this._win.document);
        sel.collapse(sel.focusNode, sel.focusOffset);
      }  
    }
  }
}
(In reply to kernp25 from comment #31)
> Or maybe make auto copy intelligent/smart?
> 
> Look at my addon:
> https://addons.mozilla.org/de/firefox/addon/quick-copy-2/
> 
> This does only copy text if you hold down the mouse for 0.8 secs.
> When its copied, the selection will be collapsed.
> So, the user can easily see that the text is copied.

Such intelligent behavior is a little bit hacky and may make creating automating tests difficult. I strongly recommend that we should not do that for this. If it were worthwhile to do that, such feature should be enabled in default settings. Probably, if you do that in Core, you will struggle with a lot of regression bugs like me :-(

And I forgot to refer our coding rules.
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Control_Structures

Please do not forget to put {} to every block even if its line is only one.

Comment 35

5 years ago
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #30)
> Comment on attachment 8434121 [details] [diff] [review]
> enable-auto-copy-on-all-platforms.patch
> 
> This patch may destroy global clipbord data unexpectedly. I guess that some
> users who don't understand the pref well have already changed the pref true.
> Then, such users would file INVALID or WONTFIX bugs.

I think we can avoid this by adding a new setting to the setting window.
e.g. settings > advanced > General tab > Accessibility or maybe add it to Browsing?
https://support.mozilla.org/en-US/kb/advanced-settings-browsing-network-updates-encryption#w_general-tab

Because now this feature only can be changed if you going to about:config
So, if the user don't want this feature, he can easily turn this feature off in the firefox settings dialog.

I think also if the users dont want this feature,
then we add this clipboard.autocopy pref to the settings of firefox

Comment 36

5 years ago
> if (sUseGlobalClipboard) {
>   return nsCopySupport::HTMLCopy(... nsIClipboard::kGlobalClipboard);
> }
> 
> if (!sSelectionClipboardSupported) {
>   return NS_OK;
> }
> 
> 

But what if sUseGlobalClipboard is false and sSelectionClipboardSupported is also false here
then the selection listener is useless here i think.
Maybe we check this before we adding the selection listener?
(In reply to kernp25 from comment #35)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #30)
> > Comment on attachment 8434121 [details] [diff] [review]
> > enable-auto-copy-on-all-platforms.patch
> > 
> > This patch may destroy global clipbord data unexpectedly. I guess that some
> > users who don't understand the pref well have already changed the pref true.
> > Then, such users would file INVALID or WONTFIX bugs.
> 
> I think we can avoid this by adding a new setting to the setting window.
> e.g. settings > advanced > General tab > Accessibility or maybe add it to
> Browsing?
> https://support.mozilla.org/en-US/kb/advanced-settings-browsing-network-
> updates-encryption#w_general-tab
> 
> Because now this feature only can be changed if you going to about:config
> So, if the user don't want this feature, he can easily turn this feature off
> in the firefox settings dialog.


Like this feature should not be added to Preference dialog. It's enough to exist only in about:config. The default settings should honor the standard behavior in each platform as far as possible. Anyway, it should be a bug of Firefox, not Core.

(In reply to kernp25 from comment #36)
> > if (sUseGlobalClipboard) {
> >   return nsCopySupport::HTMLCopy(... nsIClipboard::kGlobalClipboard);
> > }
> > 
> > if (!sSelectionClipboardSupported) {
> >   return NS_OK;
> > }
> > 
> > 
> 
> But what if sUseGlobalClipboard is false and sSelectionClipboardSupported is
> also false here
> then the selection listener is useless here i think.
> Maybe we check this before we adding the selection listener?

Yes, it's ideal behavior. But if you want to change the behavior as ideal, we need to hack around here:
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsSelection.cpp#341
And also you need to remove the selection listener *when* the pref is disabled. Additionally, you need to add the selection listener *when* the pref is disabled. If you want to do it, I don't disagree. But I think that it's excessive implementation because these prefs must not be changed many times. Therefore, I believe that it's enough to check current settings only in selection listener. This only detects disabling the prefs.

Comment 38

5 years ago
But i really think if you are not going to support other platforms, then we must really remove this 'useless' pref.(Why this pref was ever added?)
I found this: 
https://bugzilla.mozilla.org/show_bug.cgi?id=41127

> The "autocopy" feature is expected only on UNIX systems. But a pref will
> allow UNIX users to turn it off and Windows/Mac users to turn it on if they
> want it.
LOL!!!

This doesn't work in Windows/Mac anyway!!!

http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsSelection.cpp#341

My suggestion would be instead of checking the pref, check if the system supports the selection clipboard and then add the selection listener.

Replace this:
>   // Check to see if the autocopy pref is enabled
>   //   and add the autocopy listener if it is
>   if (Preferences::GetBool("clipboard.autocopy")) {

with:
>   // get clipboard
>   nsCOMPtr<nsIClipboard> clipboard(do_GetService(kCClipboardCID, &rv));
>   NS_ENSURE_SUCCESS(rv, rv);
>
>   // check whether the system supports the selection clipboard or not.
>   bool selectionSupported;
>   rv = clipboard->SupportsSelectionClipboard(&selectionSupported);
>   NS_ENSURE_SUCCESS(rv, rv);
>
>   // if the system supports it then add our autocopy listener
>   if (selectionSupported) {

But i really think if you are not going to implement this feature for other platforms you must remove this pref.
Because this pref is useless (doesn't do anything in windows/mac).

And if a user wants this functionality, he can already install an addon for this.
There are already addons that adds this functionality.

https://addons.mozilla.org/en-US/firefox/search/?q=auto+copy&platform=windows&appver=28.0
Wait, I don't understand why you want to remove the pref.

It should stay there for Linux/UNIX users turning it off.

And you're trying to implement this on all platforms, isn't it?

But the important issue of your patch is, if a user has already changed "clipboard.autocopy" to true and he/she forgot it, then, landing your current patch causes destroying clipboard data of such user when he/she just selects text. My suggestion is for preventing such critical dataloss. You are really trying to add a *new* feature and *new* feature should be enabled by new pref because any users don't accept the new feature explicitly *yet*.

Comment 40

5 years ago
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #30)
> Comment on attachment 8434121 [details] [diff] [review]
> enable-auto-copy-on-all-platforms.patch
> 
> This patch may destroy global clipbord data unexpectedly. I guess that some
> users who don't understand the pref well have already changed the pref true.
> Then, such users would file INVALID or WONTFIX bugs.
> 
> I'd like to suggest that we should add new bool pref like
> "clipboard.autocopy.allow_to_use_global_clipboard".
> 
> Then, this patch can be safer and simpler.

Isn't this useless?
I'm asking because if a windows users has set this clipboard.autocopy to true
but setting the clipboard.autocopy.allow_to_use_global_clipboard to false then the auto copy won't work
because in windows there is only global clipboard.

Maybe we only add this sSelectionClipboardSupported?
(In reply to kernp25 from comment #40)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #30)
> > Comment on attachment 8434121 [details] [diff] [review]
> > enable-auto-copy-on-all-platforms.patch
> > 
> > This patch may destroy global clipbord data unexpectedly. I guess that some
> > users who don't understand the pref well have already changed the pref true.
> > Then, such users would file INVALID or WONTFIX bugs.
> > 
> > I'd like to suggest that we should add new bool pref like
> > "clipboard.autocopy.allow_to_use_global_clipboard".
> > 
> > Then, this patch can be safer and simpler.
> 
> Isn't this useless?
> I'm asking because if a windows users has set this clipboard.autocopy to true
> but setting the clipboard.autocopy.allow_to_use_global_clipboard to false
> then the auto copy won't work
> because in windows there is only global clipboard.

Yes, it is what is my intention.

> Maybe we only add this sSelectionClipboardSupported?

As I said in my previous comment, your new feature uses global clipboard. I.e., not emulating Linux/UNIX behavior perfectly. In other words, your new feature is similar to the auto copy of Linux/UNIX but actually different. Therefore, even if users have already set clipboard.autocopy true, it does *not* mean that they accept your new feature. Adding the new pref works as warning message by its name. I believe that this is important.
And perhaps, "clipboard.autocopy.allow_to_use_global_clipboard" should be "clipboard.autocopy.use_global_clipboard" because it changes the behavior on Linux/UNIX too.

Comment 43

5 years ago
What is bz saying about this?

Comment 44

5 years ago
Is this pref clipboard.autocopy.use_global_clipboard really needed?

If i going to about:config i will get a warning dialog if the general.warnOnAboutConfig pref is set to true.

So, if the user is clicking the "I'll be careful, I promise!" button then i think the user knows what he is doing.

Comment 45

5 years ago
Or maybe we change the name of the clipboard.autocopy to maybe clipboard.autocopyonselection?

Comment 46

5 years ago
So, the user can see what this pref is doing.
(In reply to kernp25 from comment #44)
> Is this pref clipboard.autocopy.use_global_clipboard really needed?

I believe so. See comment 41.

> If i going to about:config i will get a warning dialog if the
> general.warnOnAboutConfig pref is set to true.
> 
> So, if the user is clicking the "I'll be careful, I promise!" button then i
> think the user knows what he is doing.

It doesn't make sense because your change will come after changing the pref. Some users may keep the changed prefs due to no effect. However, your patch will destroy clipboard data. Some of them may hate the behavior and probably be surprised since suddenly the regression starts. I guess that most users of them doesn't suspect themselves because the behavior change comes from Gecko, not changing prefs by themselves.

(In reply to kernp25 from comment #45)
> Or maybe we change the name of the clipboard.autocopy to maybe
> clipboard.autocopyonselection?

I also don't like the pref name. However, if we change it, some Linux/UNIX users who are using it for turning off the auto copy is re-enabled. I.e., we need migration code for this. However, runtime migration wastes both running cost and binary size.

(In reply to kernp25 from comment #46)
> So, the user can see what this pref is doing.

I hope that MozillaZine will explain the pref meaning ;-)
> How do you think, bz?

I agree that having the pref suddenly start doing something when it didn't use to is pretty weird.

If I were designing this from scratch, I'd have a pref with three values: disable, enable, system default.

Given the existence of profiles with this pref already set, I think the simplest way to not break people who have the pref set already (by either making it suddenly do stuff or renaming it) is to have two prefs.  The old one, if set false means "disable".  Otherwise it means check the new one.  The new one could be called clipboard.autocopy.force-enable or whatever and if true would enable, else not.  

That would not require runtime migration bits, at least.
Thank you, bz. It sounds good to me.

kernp25: Either of approach is okay. Could you attach a new patch and request review to me?
Attachment #8434121 - Flags: feedback?(bzbarsky)
Comment on attachment 8434121 [details] [diff] [review]
enable-auto-copy-on-all-platforms.patch

When you attach a new patch, please request review again.
Attachment #8434121 - Flags: review?(masayuki) → review-

Comment 51

3 years ago
Posted patch bug-160200-fix.patch (obsolete) — Splinter Review
Attachment #8434121 - Attachment is obsolete: true
Attachment #8780059 - Flags: review?(masayuki)
Hmm, looks like that it might conflict with the patches for bug 1261299. I think that you should create the patch after the patches are landed.

Updated

a year ago
Attachment #8780059 - Attachment is obsolete: true
Attachment #8780059 - Flags: review?(masayuki)
You need to log in before you can comment on or make changes to this bug.