Page MenuHomePhabricator

Make header texts in diff view non-copyable
Closed, ResolvedPublic

Description

by using e.g. user-select: none;

Event Timeline

WMDE-Fisch moved this task from Proposed to Sprint ready on the WMDE-QWERTY-Team board.

Change 322922 had a related patch set uploaded (by WMDE-Fisch):
Make header texts in diff view non-copyable

https://gerrit.wikimedia.org/r/322922

Change 322922 merged by jenkins-bot:
Make header texts in diff view non-copyable

https://gerrit.wikimedia.org/r/322922

The patch merged prevents users from selecting the header text directly. And when the text can't be selected it also can't be copied - in theory.

But if you select the selectable text before a header and drag the selection it to text underneath the header, the text in between will also be selected and therefore can also be copied. ( see screenshot ). Firefox seems to be the only browser that does not select the text in that case.

So you still could copy the text. This could be fixed by using CSS pseudo elements to display the text [1]. Then the text really can't be selected or copied. The only concern there would be accessibility. Although most modern browsers and screen readers seem to be able deal with that nowadays there are still exceptions [2].

So last option some JS magic? Or keep it like it is now?

Request for comments @Bmueller @Jan_Dittrich @Addshore @Tobi_WMDE_SW

[1] https://danoc.me/blog/css-prevent-copy/
[2] http://www.powermapper.com/tests/screen-readers/content/css-generated-content/

We might actually *want* the headline in there, which is, if the user copies mine and their version. Having an indicator where which block starts makes sense.
If you look at the way git does it, I wonder if it would not be cool to have something like "-----MINE------" copied (but thats just an idea)

Where it is annoying, I assume, if you want to copy a part of an unchanged block and want to take the their block along. On the other hand, you may want to copy the mine, and no matter what we do with the headlines, we will always have the their block with it (because it comes before mine.

So:

  • I assume the current way it not so bad.
  • It is complicated, and it seems that making headlines unselectable does not resolve the problem of having text which we don't want.

I've played around a bit with this and like @WMDE-Fisch said, it seems that the user-select attribute is still quite buggy in some browsers. It works in Gecko as expected, but in WebKit you can accidentally select non-selectable text by selecting elements around that text. A nice example can be found here.
So, I'm wondering whether we're confusing users more by introducing this feature. E.g. in Chrome the text looks like it is not selected, but it gets copied which feels weird.
I would go for CSS pseudo elements or abandon the feature completely.

or abandon the feature completely.

No strong objection. Without testing we do not know anyway.

I wonder if we should document this open problem and ask users about it when we are in beta (explicitly asking for behavior, like "Does this [unwanted selecting] happen to you?")

Change 326136 had a related patch set uploaded (by Andrew-WMDE):
Used pseudo elements to prevent copying

https://gerrit.wikimedia.org/r/326136

Change 328386 had a related patch set uploaded (by Andrew-WMDE):
Implemented feedback regarding pseudo elements to prevent copying

https://gerrit.wikimedia.org/r/328386

Change 328386 abandoned by Andrew-WMDE:
Implemented feedback regarding pseudo elements to prevent copying

https://gerrit.wikimedia.org/r/328386

Change 326136 merged by jenkins-bot:
Used pseudo elements to prevent copying

https://gerrit.wikimedia.org/r/326136

Tobi_WMDE_SW closed this task as Resolved.Jan 10 2017, 10:43 AM
Tobi_WMDE_SW moved this task from Done to Demoed on the WMDE-QWERTY-Team board.Jan 18 2017, 3:17 PM