166235 - |-moz-user-select = "none"| should prevent copying to clipboard

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[Phil Schwartau]

The CSS attribute |-moz-user-select| can be set to "none"
to prevent the user from being able to select text.

Visually, this looks like it's working. But if you copy to the
clipboard, this text appears when you paste into a text editor.


STEPS TO REPRODUCE:

1. Load the testcase I will attach below
2. It contains four cells of text
3. One of the four cells has |-moz-user-select| set to "none" 
4. Try to copy the entire table to the clipboard
5. You will see three of the four cells highlighted in blue
6. So far, so good
7. But paste into any text editor
8. BUG: you see the contents of all four cells!

Comment 1

[Phil Schwartau]

Attachment: Reduced testcase

Comment 2

[Geoff Lankow (:darktrojan)]

Attachment: patch v1

Ok, my C++ is horrible, I haven't written any tests, and this doesn't do -moz-user-select: -moz-none, BUT it does work! Let's see if this patch attracts any attention.

Comment 3

[Geoff Lankow (:darktrojan)]

Attachment: patch v2

Outstanding issues:
1. Unselectable text is not copied when selected text is dragged and dropped, but the d/d preview image still contains it. This'll be a separate bug but I'm mentioning it here because the behaviour is now inconsistent with the UI.
2. Calling toString() on a Selection or Range object contains the unselectable text.
3. I found some selection issues while writing tests that aren't caused by this patch. Checking if these have bug numbers already.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 424557 [details] [diff] [review]
patch v2

Mats, can you take a look at this?

Also, I'm not sure what I think of giving untrusted code the ability to easily prevent copying...  Then again, maybe it has that anyway?

Comment 5

[Geoff Lankow (:darktrojan)]

(In reply to comment #4)
> Also, I'm not sure what I think of giving untrusted code the ability to easily
> prevent copying...  Then again, maybe it has that anyway?

It's probably not the first time, but I can't think of another example. If necessary we could restrict this to only certain types of documents (e.g. -moz-user-select is used for displaying XML, and I'm thinking of using it in a fix for bug 246620).

Comment 6

[Boris Zbarsky [:bzbarsky]]

ccing some folks who might be interested in that issue.

Comment 7

[Neil Deakin]

We may also want to consider the long-standing issue of nodes without frames being copied/selected.

I think if we had a problem with untrusted code disabling something, it might make more sense to disable -moz-user-select.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Neil, see bug 39098 (has patch) for the first paragraph of comment 7.

Comment 9

[Mats Palmgren (inactive)]

(see bug 477152 for the second paragraph of comment 7)

Comment 10

[Mats Palmgren (inactive)]

Comment on attachment 424557 [details] [diff] [review]
patch v2

The intent of -moz-user-select:none, as far as I understand it, is to
prevent content from being *selected by the user* using the mouse/
kbd commands/context-menu etc.  The bug is that this content shouldn't
be in the Selection in the first place.  Also, any content added to the
Selection *by script* should still be copied even if it's styled with
-moz-user-select:none.

This patch does not prevent selecting non-selectable content, it just
avoids copying that content from the Selection.
I guess we could take it as wallpaper for now, since it's better than
the current behavior.

>+  if (mIsCopying) {

You should remove this to make this block unconditional.
It makes Selection.toString() also exclude the non-selectable content.
(don't forget to fix the testcase accordingly)

With the above nit, r=mats


Regarding the outstanding issues (comment 3):
1. filed as bug 556432
2. is fixed by the above nit
3. please file new bugs as needed

Comment 11

[Geoff Lankow (:darktrojan)]

Attachment: patch v3

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/mozilla-central/rev/2b560e956063

Comment 13

[Timothy Nikkel (:tnikkel)]

This caused bug 558610.

Comment 14

[Mats Palmgren (inactive)]

The regression was fixed in bug 558726.