Closed Bug 166235 Opened 22 years ago Closed 14 years ago

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

Categories

(Core :: DOM: Selection, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a5

People

(Reporter: pschwartau, Assigned: darktrojan)

References

Details

(Keywords: testcase)

Attachments

(2 files, 2 obsolete files)

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!
Attached file Reduced testcase
Blocks: 302761
Assignee: mjudge → nobody
QA Contact: pmac → selection
Keywords: testcase
Hardware: x86 → All
Attached patch patch v1 (obsolete) — Splinter Review
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.
Attached patch patch v2 (obsolete) — Splinter Review
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.
Assignee: nobody → geoff
Attachment #424532 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #424557 - Flags: review?(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?
Attachment #424557 - Flags: review?(bzbarsky) → review?(matspal)
(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).
ccing some folks who might be interested in that issue.
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.
Neil, see bug 39098 (has patch) for the first paragraph of comment 7.
Blocks: 246620
(see bug 477152 for the second paragraph of comment 7)
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
Attachment #424557 - Flags: review?(matspal) → review+
Attached patch patch v3Splinter Review
Attachment #424557 - Attachment is obsolete: true
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/2b560e956063
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Depends on: 558726
This caused bug 558610.
Depends on: 558610
The regression was fixed in bug 558726.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: