Created attachment 318506 [details] [review]
pasteManager: WIP: Allow adding prefix to pasted text

(In reply to Allan Day from comment #0)
> I'm attaching an experimental mockup for how this issue could be resolved.
> One thing that's missing from it is a warning that pastes are public -
> that's something it'll need.

I don't think a warning after uploading the text is enough - it just allows the user to not post the link to the channel, but the paste will already be publicly available at https://paste.gnome.org/all.
(Also if possible, I'd prefer something that doesn't require complex custom widgets, but that's secondary).

I'm attaching an old work-in-progress patch I did a couple of month ago, but didn't pick up because I didn't like it too much - might still be useful as a base.

(In reply to Florian Müllner from comment #1)
...
> > I'm attaching an experimental mockup for how this issue could be resolved.
> > One thing that's missing from it is a warning that pastes are public -
> > that's something it'll need.
> 
> I don't think a warning after uploading the text is enough

Right - the warning would have to be shown before you paste. It could be a one time thing, the first time you use the paste integration feature.

> (Also if possible, I'd prefer something that doesn't require complex custom
> widgets, but that's secondary).

The main requirement is to show a placeholder in the text entry. Let me know if there's a way to do that with the standard entry widget, even if it's just a non-editable text placeholder.

Created attachment 318547 [details]
a proposed iteration on allans experimental mockup


Attaching a proposed iteration of the experimental mockup.

(In reply to Allan Day from comment #2)
> (In reply to Florian Müllner from comment #1)
> ...
> > > One thing that's missing from it is a warning that pastes are public -
> > > that's something it'll need.
> > I don't think a warning after uploading the text is enough
> 
> Right - the warning would have to be shown before you paste. It could be a
> one time thing, the first time you use the paste integration feature.

Perhaps a display popover shown only the first time could work? The behavior I'm describing in this attached mockup is a bit disruptive.. We need to ensure the user has read the warning and isn't left with a bad surprise, though.

Secondly, I think in general we should be careful with uploading anything to the public before the user has given his final consent (by hitting enter). I don't think we can trust that what the user intends to Ctrl+V is what ends up being pasted. Having a preview or indicating the amount of lines might go some way towards prevent these issues.


> > (Also if possible, I'd prefer something that doesn't require complex custom
> > widgets, but that's secondary).
> 
> The main requirement is to show a placeholder in the text entry. Let me know
> if there's a way to do that with the standard entry widget, even if it's
> just a non-editable text placeholder.

I did some visuals in the proposal that shouldn't require custom widgets as far as I can see. Let me know if any of it works for you.

(In reply to Bastian Ilsø from comment #3)
> Secondly, I think in general we should be careful with uploading anything to
> the public before the user has given his final consent (by hitting enter). I
> don't think we can trust that what the user intends to Ctrl+V is what ends
> up being pasted. Having a preview or indicating the amount of lines might go
> some way towards prevent these issues.

I agree. I don't think confirmation is about whether to use a public paste service with the network/channel, but about whether to upload the pasted text. Being fine with uploading patches doesn't mean being fine with uploading banking information as well :-)


> > The main requirement is to show a placeholder in the text entry. Let me know
> > if there's a way to do that with the standard entry widget, even if it's
> > just a non-editable text placeholder.
> 
> I did some visuals in the proposal that shouldn't require custom widgets as
> far as I can see.

No, it's not really much different from Allan's proposal. An entry can have:
 - an optional icon at the start
 - one line of editable text (with a single style, no markup)
 - an optional icon at the end

We could use a TextView instead that does allow implementing either proposal, but we'd need to hack it up so that <enter> sends the message rather than starting a new line. Plus, if we want to have clickable icons there (for instance for emojis as in some of the mockups), we'd need custom code for that as well.

Created attachment 318947 [details]
mockups with confirmation for each upload

(In reply to Florian Müllner from comment #4)
> (In reply to Bastian Ilsø from comment #3)
> > Secondly, I think in general we should be careful with uploading anything to
> > the public before the user has given his final consent (by hitting enter). I
> > don't think we can trust that what the user intends to Ctrl+V is what ends
> > up being pasted. Having a preview or indicating the amount of lines might go
> > some way towards prevent these issues.
> 
> I agree. I don't think confirmation is about whether to use a public paste
> service with the network/channel, but about whether to upload the pasted
> text. Being fine with uploading patches doesn't mean being fine with
> uploading banking information as well :-)

The question is whether someone will remember that the paste service is public after being told once. One possibility here could be to have a prominent warning dialog the first time someone uses the paste service, which would draw more attention to the privacy issue.

Alternatively, you could ask for confirmation each time. It's probably not terrible to do it this way, although it will obviously get in the way a bit. 

I've attached some mockups for how this could look.

(In reply to Allan Day from comment #5)
> The question is whether someone will remember that the paste service is
> public after being told once. One possibility here could be to have a
> prominent warning dialog the first time someone uses the paste service,
> which would draw more attention to the privacy issue.

I see two issues with that:
 (1) this addresses the privacy issue of uploading information from an
     internal/private channel to a public server, but not the issue of
     accidentally uploading the wrong information
 (2) IMHO, if we ask users to make a decision, we also need to offer
     them an obvious way to revisit it in case they change their mind


> Alternatively, you could ask for confirmation each time. It's probably not
> terrible to do it this way, although it will obviously get in the way a bit.

Yeah, that's what I really like about the current approach, but we cannot maintain that if we want separate confirmations for uploading pasted text and sending the message as a whole, which I think makes sense. Still, if we make it two <enter> presses instead of one, I don't think it's too bad.

Would it make sense to implement the feature without using complex custom widgets first by:
 - switching to the current paste UI when pasting
 - make the buttons insensitive while uploading
 - switch back to the entry when done and append
   the URL to the text
 - send everything as normal message on <enter>

(In reply to Florian Müllner from comment #6)
> (In reply to Allan Day from comment #5)
> > The question is whether someone will remember that the paste service is
> > public after being told once. One possibility here could be to have a
> > prominent warning dialog the first time someone uses the paste service,
> > which would draw more attention to the privacy issue.
> 
> I see two issues with that:
>  (1) this addresses the privacy issue of uploading information from an
>      internal/private channel to a public server, but not the issue of
>      accidentally uploading the wrong information

A one time warning is inevitably not going to be as effective at preventing errors than a warning that's shown every time. Of course, warnings or requiring confirmation isn't that good at preventing mistakes in general.

>  (2) IMHO, if we ask users to make a decision, we also need to offer
>      them an obvious way to revisit it in case they change their mind

Some actions, like sending something, are irreversible. One possibility here is to delay actually sending the data (like Gmails's Undo Send [1]). Although it is a bit trickier in this case because we need to get the link back from the server. Although, that could potentially delay being able to post.

Another option would be to allow undo by deleting the paste from the server, if such a thing is possible.

> > Alternatively, you could ask for confirmation each time. It's probably not
> > terrible to do it this way, although it will obviously get in the way a bit.
> 
> Yeah, that's what I really like about the current approach, but we cannot
> maintain that if we want separate confirmations for uploading pasted text
> and sending the message as a whole, which I think makes sense.

Two confirmations sounds like overkill to me, to be honest. It could get annoying pretty quickly.

> Would it make sense to implement the feature without using complex custom
> widgets first by:
>  - switching to the current paste UI when pasting
>  - make the buttons insensitive while uploading
>  - switch back to the entry when done and append
>    the URL to the text
>  - send everything as normal message on <enter>

That would certainly be better than what we have now, and would fix the bug. The one thing it wouldn't do which I had in my mockups is being able to edit the message while upload is happening, but maybe that's OK.

[1] https://support.google.com/mail/answer/1284885?hl=en

(In reply to Allan Day from comment #7)
> > Would it make sense to implement the feature without using complex custom
> > widgets first by:
> >  - switching to the current paste UI when pasting
> >  - make the buttons insensitive while uploading
> >  - switch back to the entry when done and append
> >    the URL to the text
> >  - send everything as normal message on <enter>
> 
> That would certainly be better than what we have now, and would fix the bug.
> The one thing it wouldn't do which I had in my mockups is being able to edit
> the message while upload is happening, but maybe that's OK.

One way to allow to edit the message while upload is happening, if we use the confirmation popover from your mockup to ask for permission. We could then use the unicode LINK character (U+1F517) [1] to indicate where the link will appear (the unicode symbol would need to be added to Cantarell of course).

[1]: http://www.fontspace.com/unicode/char/1F517-link-symbol


..I threw some screenshots together in a poor animation for clarification..
https://youtu.be/g5WM28quNyA

(In reply to Allan Day from comment #7)
> Some actions, like sending something, are irreversible. One possibility here
> is to delay actually sending the data (like Gmails's Undo Send [1]).
> Although it is a bit trickier in this case because we need to get the link
> back from the server. Although, that could potentially delay being able to
> post.

(requires custom widgetry, so trickier:)
Another option I can think of is inserting a "x lines of pasted text" placeholder (that can be selected/deleted as a single character). If the placeholder is present when the message is sent, we upload the text and insert the actual URL before sending actually sending the message.


> Another option would be to allow undo by deleting the paste from the server,
> if such a thing is possible.

No, the service doesn't allow that, and probably never will - unless we'd be OK with allowing random people to delete other people's pastes, we'd need some way to authenticate, which kind of defeats the purpose of an open-to-all service.


> Two confirmations sounds like overkill to me, to be honest. It could get
> annoying pretty quickly.

Fair enough, but doesn't that contradict:

> > Would it make sense to implement the feature without using complex custom
> > widgets first by:
> >  [...]
> 
> That would certainly be better than what we have now, and would fix the bug.

There are two confirmations in that proposal, one for the paste and one for sending the message ...

*** Bug 758020 has been marked as a duplicate of this bug. ***

Created attachment 321038 [details] [review]
pasteManager: Remove upload notification

We are about to rework paste service integration a bit, and the
upload notification gets in the way of some necessary refactoring.
As it will eventually be replaced with a different indication anyway,
just remove it.

Created attachment 321039 [details] [review]
entryArea: Split out a _setPasteContent() method

More than avoiding a bit of code duplication, this gives us a central
place to manage switches between entry and paste confirmation UI.

Created attachment 321040 [details] [review]
entryArea: Use pasteManager directly to handle pastes

Our current paste handling works pretty much in a fire-and-forget way:
the pasted content is handed over to the pasteManager which will take
care of uploading it and sending the resulting URL to the current channel.
As a result, URLs are always inserted as standalone message with no way
for users to add additional context like highlights.
To address this, the entry will need access to the returned URL before
the message is send - achieving this using actions that don't have
a return value would be messy, so simply cut out the middle man and
use the pasteManager singleton directly.

Created attachment 321041 [details] [review]
pasteManager: Make pasteImage() take a pixbuf

Now that we don't have to go through GVariant, it is cleaner to keep
passing around the actual pixbuf and only serialize it once we hit
imgurPaste().

Created attachment 321042 [details] [review]
entryArea: Take over processing of the result URL

In order to allow users to send paste URLs as part of normal messages
rather than stand-alone, the result URL should be handled by the
appropriate entry rather than generically by the paste manager.

Also removing code duplication between text- and image pastes is a nice
side effect.

Created attachment 321043 [details] [review]
entryArea: Insert paste URL instead of sending it directly

It usually makes sense to provide some context when sending a paste
URL, so allow that by inserting the returned paste URL into the
entry instead of sending it directly. Pasting content as URL is
now a two-step operation - confirming the upload and sending the
message - but hopefully that'll just be minor annoyance compared
to the usefulness of the feature.

Created attachment 321044 [details] [review]
entryArea: Use paste-confirmation area to indicate upload progress

Now that we insert paste URL into the entry rather than sending them
directly, we don't want the current entry text to change while the
upload is in progress - we'd just end up with situations where the
insertion interferes with what the user is currently typing.

To prevent this, we only switch back to the entry after the upload
has finished, which makes the paste-confirmation area available for
replacing the old upload notification we removed earlier.

Attachment 321038 [details] pushed as 46b9e2a - pasteManager: Remove upload notification
Attachment 321039 [details] pushed as b913300 - entryArea: Split out a _setPasteContent() method
Attachment 321040 [details] pushed as e0b15c9 - entryArea: Use pasteManager directly to handle pastes
Attachment 321041 [details] pushed as 6c5231b - pasteManager: Make pasteImage() take a pixbuf
Attachment 321042 [details] pushed as ad73a40 - entryArea: Take over processing of the result URL
Attachment 321043 [details] pushed as 4a920bd - entryArea: Insert paste URL instead of sending it directly
Attachment 321044 [details] pushed as cad0a73 - entryArea: Use paste-confirmation area to indicate upload progress