Closed Bug 1572648 Opened 5 years ago Closed 2 years ago

Support rich embedding (previews) of links in composed content (Twitter Card, Open Graph data)

Categories

(Thunderbird :: Message Compose Window, enhancement)

enhancement

Tracking

(thunderbird_esr91 wontfix, thunderbird_esr102 fixed, thunderbird102 fixed)

RESOLVED FIXED
102 Branch
Tracking Status
thunderbird_esr91 --- wontfix
thunderbird_esr102 --- fixed
thunderbird102 --- fixed

People

(Reporter: mkmelin, Assigned: mkmelin)

References

(Depends on 1 open bug, )

Details

(Keywords: leave-open)

Attachments

(3 files)

We should enable making the content of mails from our users look great. Other platforms usually enhance embedded links by providing previews or direct access.

There are standard ways to do this:

Open Graph and Twitter card is basically supported on all news sites (etc.) due to the the size of Facebook and Twitter. Youtube and many other media giants support oEmbed.

We should definitely put some resources on this.

I was surprised to see this filed as a compose item (and not a display item). Are you envisioning this as something that pops up when a user tries to paste in a Twitter link that asks "Do you want to display this is a pretty fashion?", if they click yes Thunderbird then inserts all the proper HTML and such? (Ignore the exact UX, just trying to illustrate the idea.)

I've wanted this for chat, although I can't find the bug at the moment, but had always assumed it would be on display (e.g. a user sends us a Twitter link and the Thunderbird UI finds it and embeds the pretty content somehow).

The implementation should make it easy to hook up both cases. I was primarily thinking about it for compose due to previous experience with it - BuddyPress/Wordpress uses oEmbed for writing statuses/posts. Also Open Graph includes in Facebook does the lookup on compose (it seems).

Doing it for the compose stage should make it easier to create pretty things when you compose, and gives more control.

For compose, UX wise I think it could work pretty much like when you create a message with link on Facebook: It adds the card below and you can inspect it and remove if you want (with an X on the corner).

For receiving, UX is more difficult and needs more consideration on what to show and when. Not to mention security/privacy is more of an issue than when it's a link you yourself added to a message.

We should enable making the content of mails from our users look great. Other platforms usually enhance embedded links by providing previews or direct access.

Note that "remove it if you want" is still too intrusive for users not interested in the feature.

And - if this feature is introduced, I ask that there be:

  • At least 3 settings: Disable, preview in a pop-up, preview in message body.
  • A separate default setting regarding the feature for text and HTML messages (and other types if those become supported, e.g. Markdown).

Finally, when this is disabled, that the link contents will not be silently collected and merely hidden. i.e. that we not introduce additional network activity unless this feature is actually desired.

I spend some time investigating this. We won't do anything with oEmbed: it's trivial to add, but unfortunately, the content provided does not really work in a mail setting. Content is in an iframe, which doens't work out nicely... and even when it would work out nicely some sites (YouTube) don't allow viewing the embedded content from non-http origin.
And oEmbed content mostly needs JavaScript as well...

Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Summary: support rich embedding (previews) of links in composed content (oEmbed, Twitter Card, Open Graph data) → support rich embedding (previews) of links in composed content (Twitter Card, Open Graph data)

(In reply to Magnus Melin [:mkmelin] from comment #5)

We won't do anything with oEmbed: it's trivial to add, but unfortunately, the content provided does not really work in a mail setting. Content is in an iframe, which doens't work out nicely... and even when it would work out nicely some sites (YouTube) don't allow viewing the embedded content from non-http origin.
And oEmbed content mostly needs JavaScript as well...

I agree that the above is true for the "rich" oEmbed type (and probably "video"). The "photo" type is just a raw URL and metadata, so could probably do something with that! It does seems to be used quite a bit less, in my experience. (There's also a "link" type, which doesn't seem very useful.)

Yes, but photo is already covered by the the others, which are much more common.

Is there a potential privacy issue with this feature? I'm not completely sure I have the right understanding, but I thought I should flag it just in case.

Looking at the current work-in-progress patch, if you paste any plain text that starts with https:// then the link will be fetched. The fetch is performed regardless of the rest of the link. So pasting any link will (silently) fetch the response in the background: you only see an effect if preview data was available.

If you copy the link from a regular browser that uses the same network as thunderbird, then this will just be an extra network request that was already done by the browser. However, for a user at risk of their traffic being monitored this might be an issue: whilst network requests to their email servers or even thunderbird links are safe or trusted, visible network requests to specific urls might put them at risk, or be considered exposing private information. For example, consider a user that trusts their email server (or uses PGP), connecting to their server does not put them at risk, but they do not trust their ISP (internet service provider) with all the urls they paste into their emails. For such a user, copying a link from the Tor Browser, or some other browser that redirects requests, or even just copying a link from a local document, could then be exposed by thunderbird to the ISP.

I don't know much about this area, so maybe there is some factor that trivialises the problem. But perhaps this should be considered before we enable this feature by default.

Yes, that is basically correct. Some people may not want to have it, so we have a pref to turn it off.
I would see privacy implications as very very minor though. I mean, were did you get the link in the first place? Almost exclusively from already having opened them in the browser. Since we only do https links, an ISP can figure out the host but not the actual url. If someone is worried about their ISP being able to figure out which hosts they connect to, they really need to be using a VPN to begin with.
Moreover, pasting a link is a very active action: you're going to send the link and want someone else to open it. Why would you do that if you're not ok with "opening" it yourself...

(In reply to Magnus Melin [:mkmelin] from comment #10)

I would see privacy implications as very very minor though. I mean, were did you get the link in the first place? Almost exclusively from already having opened them in the browser.

Yes, for most users this will be true, but I'm considering users that might be at risk. For such users the link could have been obtained from a secured channel. E.g. it could have been obtained from Tor Browser, or a local document.

Since we only do https links, an ISP can figure out the host but not the actual url. If someone is worried about their ISP being able to figure out which hosts they connect to, they really need to be using a VPN to begin with.

I think there are situations where a trusted VPN cannot be safely used. E.g. people who would use bridges to connect to tor within China or Iran (and probably more places).

Moreover, pasting a link is a very active action: you're going to send the link and want someone else to open it. Why would you do that if you're not ok with "opening" it yourself...

You might be ok opening it in only a secure channel. The person you are sending the link to might not be under the same risk, or would take the same precautions of not opening it in a plain browser.

Yes, that is basically correct. Some people may not want to have it, so we have a pref to turn it off.

Could we by default have no hard preference one way or the other? Instead, in the compose window, if a user pastes a link, we show a dialog or an inline popup, or use the notification in the #compose-notification-bottom notification area, asking something like "Would you like Thunderbird to fetch <url> for a preview of its content?". With the options of "yes", "no" and "remember this for all links I paste". That way a user would be made aware of the new feature, but can still make a choice to explicitly opt in or out.

Similar to how we handle showing remote content in the messenger browser.

I think there is a fine line to walk, wrt. scaring people when there should be little concerns.

Similar to how we handle showing remote content in the messenger browser.

Not asking IS how we do it in compose atm. Try pasting content containing images etc. Those will just be downloaded and added to the content, without asking a thing.

For privacy it may be best to ask the user first? Maybe when they paste something link-looking Thunderbird can display a message like "Do you want to embed a preview for this link?". We could start there then decide about enabling by default after more security review and once it is better tested.

I agree that the risk is relatively low because the link is being followed, not shared with a third-party but I still think it shouldn't be enabled by default. A user that has been using Thunderbird for years expects that they can safely paste links into it, so we shouldn't start leaking their IP and client information to the link target.

Could we by default have no hard preference one way or the other? Instead, in the compose window, if a user pastes a link, we show a dialog or an inline popup, or use the notification in the #compose-notification-bottom notification area, asking something like "Would you like Thunderbird to fetch <url> for a preview of its content?". With the options of "yes", "no" and "remember this for all links I paste". That way a user would be made aware of the new feature, but can still make a choice to explicitly opt in or out.

Similar to how we handle showing remote content in the messenger browser.

I actually think that we may want to, at least, present a user with some kind of question like this the first time they use the feature. It's probably worthwhile to pull Alex in on this and see if we can do some user testing on a few prototypes of this.

Flags: needinfo?(alessandro)

Any prompt needs a yes, no, maybe response. Many will love it, but there will be those that have exactly the opposite reaction. Or those like me that do not want to set a preference either way, as sometimes I might want a preview and sometime not and don't want to be bugged for each link that is pasted in.

All great points, thank you all so much for raising concerns and suggestions.
Here's a list of things I think we should aim at for this feature:

  • Not enabling it by default to not automatically add "cards" preview and fetch links without the user's consent.
  • Gently exposing this feature the first time a link is added to introduce users to this new option.
  • Offer full control of future interactions so the user can decide how to handle it.

I think we can solve this by getting inspired from what Google does in their gDoc suite [see attachment], but extending it a bit to fit our needs.
Here's what I'm proposing:

  • The first time we detect a link (pref 0), we show an inline card with a fake preview (no real data fetch) asking the user something along the lines of "Do you want to replace this URL with a Preview Link?", offering 3 options:
    • "Yes, always" pref 1
    • "No, never" pref 2
    • "Ask me when hovering links" pref 3

Doing so will expose the user to this feature for the first time, and allow them to pick the outcome. Option 3 will leave things as they are, without any further prompt, unless the user specifically hovers over a link for a predefined amount of time (2 seconds?).
If option 3 was selected, the inline prompt will only have the "Yes" button to convert only the selected URL into a preview.
Changing this pref should then be possible inside the Settings > Compose section.

It's a bit more complicated in terms of development, but it can give us a first taste of what it takes to remove all the unnecessary dialogs we currently use in the composer and adopt an inline popup. We can do that in small chunks.

What do you think?
Any potential issues I'm missing?
I can do some quick mock-ups if needed.

Flags: needinfo?(alessandro)

Example of how google doc handle this.

Mock-ups might help, also if there is any way we can actually test a few users with some kind of wireframe and see what their interaction with this looks like - that might be helpful too (I can gather the people if you want to try it).

Here's a very simple interactive prototype for the Link Preview: https://app.presentator.io/#/vgfdzj34
You can click on a blank space to see which areas are interactive and can be clicked.

This is the potential workflow:

  • For the first time a link is detected, we automatically show the popup.
  • After that first exposure, we only show it if the user clicks and hover a link.
  • In the popup prompt card we offer the option to automatically add preview links (can be managed in the prefs as well).
  • Clicking on the X on the preview card will revert back to a regular link.

Thoughts?

EDIT: We can use these interactive mock-ups to gather some user feedback and see if it's intuitive enough.

Flags: needinfo?(ryan)
Flags: needinfo?(mkmelin+mozilla)

(In reply to Alessandro Castellani [:aleca] from comment #19)

Here's a very simple interactive prototype for the Link Preview: https://app.presentator.io/#/vgfdzj34
You can click on a blank space to see which areas are interactive and can be clicked.

Thanks. They look good.

The mockup is based on a twitter link, but based on the work-in-progress patch, any pasted link will be fetched to potentially show a preview. So we also need a popup that is relevant for generic links. Moreover, most links will fail to show a preview. The popup would need to make it clear that a preview will be attempted for all links, but won't necessarily succeed.

This is the potential workflow:

  • For the first time a link is detected, we automatically show the popup.

I'm not sure we would need to do this. Having the popup show on hover or click might be sufficient to make it discoverable. Moreover, since there is a good chance that the first pasted link will fail to show a preview, I'm not sure it will be the best "showcase" of the feature. I suppose we could only trigger this for a list of common url patterns where we expect previews to work.

  • After that first exposure, we only show it if the user clicks and hover a link.

Is there a way to expose it if you don't use a mouse? I can't think of an obvious way to do this within the context of an editor, especially since the popup could be disruptive. Note, we currently allow you to edit a link by pressing Ctrl+K whilst the caret is over the link.

Also, how would we handle focus on click or hover? What happens if you click the middle of a link because you want to edit it. Would you have to close the popup first, or could you immediately start typing and automatically close it?

  • In the popup prompt card we offer the option to automatically add preview links (can be managed in the prefs as well).
  • Clicking on the X on the preview card will revert back to a regular link.

This sounds good.

I have some further questions

  1. Currently, if you paste plain text that looks like a url, it will still be inserted as plain text. Is this bug going to change that so it is included the same as "Insert, Link"? To allow the preview to be fetched later, on request? This seems to be the intention in the mockup. Will there be a way to "un-link" the text to make it plain text again?
  2. Are we going to expose the preview option in the "Link Properties" dialog? That could be a means for keyboard users to use the feature. Although I think I've seen there are plans to dump the dialog.

I agree with henry. I think it might make sense to make it specific. If we preload favicons + names for a couple of sites the user might be confused why there is no logo for some sites or be concerned that we reached out to fetch the favicon before they gave permission.

I'm setting up some user testing based on what you put in presentator. I am concerned a little about discoverability, even if the pop up comes up the first time. I'm not sure some users will really understand what is going to happen if they press "yes", and once they press "yes" they may decide they want to have it come up each time, but are not sure how to choose that option at that point.

But we will see what we discover in testing.

Flags: needinfo?(ryan)

Might indeed be challenging to know when to "advertise" the feature when we don't know if it will be something reasonable we can show for it.

Maybe instead, say the 10 first times (or until action) a link is pasted, we fetch it - and if we have something showing, then we add a notification infobar telling people how to turn on and off the feature. Using a normal notification also solves keyboard and other accessibility issues.

Flags: needinfo?(mkmelin+mozilla)

(In reply to Henry Wilkes [:henry] from comment #20)

Thanks. They look good.

Thanks.

The mockup is based on a twitter link, but based on the work-in-progress patch, any pasted link will be fetched to potentially show a preview. So we also need a popup that is relevant for generic links. Moreover, most links will fail to show a preview. The popup would need to make it clear that a preview will be attempted for all links, but won't necessarily succeed.

I don't think we need to communicate this, and we should keep this popup to a minimum amount of content.
If the link doesn't have any image, we will show a card with a title and a description (body excerpt?) or whatever content is available.
I don't think we need to explain every possible scenario. If a card comes out without useful data, the user can remove that card.

I'm not sure we would need to do this. Having the popup show on hover or click might be sufficient to make it discoverable. Moreover, since there is a good chance that the first pasted link will fail to show a preview, I'm not sure it will be the best "showcase" of the feature. I suppose we could only trigger this for a list of common url patterns where we expect previews to work.

I'm okay with not showing it and just wait for it to be discoverable when the user hovers or selects a link.

Is there a way to expose it if you don't use a mouse? I can't think of an obvious way to do this within the context of an editor, especially since the popup could be disruptive. Note, we currently allow you to edit a link by pressing Ctrl+K whilst the caret is over the link.

If the caret ends up on a link, we could show it, with something like aria-live="polite" to be less intrusive. The popup should be dismissable with Esc and pressing Enter should generate the card.

Also, how would we handle focus on click or hover? What happens if you click the middle of a link because you want to edit it. Would you have to close the popup first, or could you immediately start typing and automatically close it?

The popup shouldn't interfere with the typing experience.
We should automatically close it if the user edits the link or starts typing. Also, the popup should have a small delay of maybe 2 seconds before appearing.

  1. Currently, if you paste plain text that looks like a url, it will still be inserted as plain text. Is this bug going to change that so it is included the same as "Insert, Link"? To allow the preview to be fetched later, on request? This seems to be the intention in the mockup. Will there be a way to "un-link" the text to make it plain text again?

I don't know if we want to change the behavior, but we shouldn't do it in this bug.
Let's start simple for now, and handle only links that area actually links and users specifically made them as links.
Once that works, we will explore how to deal with URLs pasted or typed as plaintext.

  1. Are we going to expose the preview option in the "Link Properties" dialog? That could be a means for keyboard users to use the feature. Although I think I've seen there are plans to dump the dialog.

We want to dump that dialog and use a similar paradigm (inline popup) to control links.
We can definitely explore the possibility of having a check to "convert this URL into a link preview".

(In reply to Kevin Cox [@kevincox:matrix.org] from comment #21)

I agree with henry. I think it might make sense to make it specific. If we preload favicons + names for a couple of sites the user might be confused why there is no logo for some sites or be concerned that we reached out to fetch the favicon before they gave permission.

My mock-up won't reflect 100% reality, especially for a first iteration.
Most likely the majority, if not all, of the preview cards will only have the descriptive text.

(In reply to Ryan Lee Sipes from comment #22)

I am concerned a little about discoverability, even if the pop up comes up the first time.

My suggestion is to keep it simple for now.
Let's focus on the first implementation, which is "Showing the popup when the hover over it".
Not super discoverable by default, but it's advisable to let this feature simmer for a bit and the slowly increase its discoverability by exposing it more aggressively later on.

I'm not sure some users will really understand what is going to happen if they press "yes", and once they press "yes" they may decide they want to have it come up each time, but are not sure how to choose that option at that point.

I might be wrong, but I don't think that will be much of a problem. In the popup there is a checkbox to "always add link previews", so if the user really likes it, after one or two times of using it, they can check that checkbox.
We can later add a toggable option in the menu bar, and also in the compose settings, allowing users to disable it if they want.

(In reply to Magnus Melin [:mkmelin] from comment #23)

Might indeed be challenging to know when to "advertise" the feature when we don't know if it will be something reasonable we can show for it.

I agree.
I'd suggest to only show that popup on link hover or selection as a first step to try this feature out and see how it's received.
We can try to expose it more aggressively later on, during the 102 cycle.

Maybe instead, say the 10 first times (or until action) a link is pasted, we fetch it - and if we have something showing, then we add a notification infobar telling people how to turn on and off the feature.

I don't think we should auto-fetch any link, not without a user's specific action.

Recommendations
We can do a lot of this on this, but I don't think we should.
For us it might be a simple and cool feature, but for many users might be very controversial and a violation of privacy, even if technically not true, but we need to be careful about users expectation.
I think as a first initial implementation for 102, we should keep it simple:

  • Don't fetch any link automatically.
  • Only show a "Wanna use a preview link?" popup when a link is clicked or hovered, with 1 second delay and no typing detected.
  • Add prefs in the Settings > Compose to "Always show a Link Preview card" toggle.
  • Add the same toggle in the View menu bar (or Edit?) in the compose window.

Updated the mock-ups to remove any Twitter reference, and added the "Edit" and "Copy" icons in the card.

(In reply to Alessandro Castellani [:aleca] from comment #25)

Updated the mock-ups to remove any Twitter reference, and added the "Edit" and "Copy" icons in the card.

I really liked the favicon personally, and given that Twitter cards are different than other types of links, I thought having it made sense.

My suggestion is to keep it simple for now.
Let's focus on the first implementation, which is "Showing the popup when the hover over it".
Not super discoverable by default, but it's advisable to let this feature simmer for a bit and the slowly increase its discoverability by exposing it more aggressively later on.

Is there any way to just put a tiny icon at the end of the link? Like a bubble with an "!" point in it or some kind of picture icon?

(In reply to Alessandro Castellani [:aleca] from comment #24)

Recommendations
We can do a lot of this on this, but I don't think we should.
For us it might be a simple and cool feature, but for many users might be very controversial and a violation of privacy, even if technically not true, but we need to be careful about users expectation.
I think as a first initial implementation for 102, we should keep it simple:

  • Don't fetch any link automatically.
  • Only show a "Wanna use a preview link?" popup when a link is clicked or hovered, with 1 second delay and no typing detected.
  • Add prefs in the Settings > Compose to "Always show a Link Preview card" toggle.
  • Add the same toggle in the View menu bar (or Edit?) in the compose window.

This sounds good. I have a few concerns with some of the details.

How will this combine with the upcoming editing popup?

We want to dump that dialog and use a similar paradigm (inline popup) to control links.

There seem to be two paths:

  1. We keep the two popups separate. I personally think it would be strange to have two different popups that relate to links. Regardless, if we do this, we need a way to activate the editing popup that doesn't overlap with the preview popup. If we stick to the proposal, this would probably be double-click (which currently already opens the link dialog). We could instead show the edit popup on a single click, and thus only show the preview on hover. But then this would raise the problem of how a keyboard user would activate the feature.
  2. Absorb the preview option into the edit popup. However, if we combine them then the current mock up would be too much to move entirely into an edit popup. In particular, the "Automatically add Preview Links when possible" checkbox would make less sense within the edit popup.

It might make sense to design the link editing popup in combination with this preview feature.

The current mockup seems more geared towards introducing a feature, than to continuously use it.

Consider a user that wants to ultimately selectively use this feature, i.e. they want to explicitly select this for some of their links. After a few times, all they want from the popup is to be able to click "Replace this link with a preview". The current mockup is a bit out of place for this usage, even though this user is going to see this popup more often than the user who selected to always show a preview.

Keyboard interaction.

If the caret ends up on a link, we could show it, with something like aria-live="polite" to be less intrusive. The popup should be dismissable with Esc and pressing Enter should generate the card.

Also, how would we handle focus on click or hover? What happens if you click the middle of a link because you want to edit it. Would you have to close the popup first, or could you immediately start typing and automatically close it?

The popup shouldn't interfere with the typing experience.
We should automatically close it if the user edits the link or starts typing. Also, the popup should have a small delay of maybe 2 seconds before appearing.

I'm not sure about this keyboard interaction. If the popup grabs focus, then its politeness won't help. If it doesn't grab focus, there is no normal way to navigate into it without disrupting the editing process because the arrow keys are used for moving the caret and tab is treated as a typed character.

Moreover, I don't think caret position is a good replacement to indicate focus or interactivity within the editor. E.g. you can use the down arrow to move to the next line and wait for your screen reader to read back the full line. The caret might happen to land on a link, but that doesn't mean the user is trying to interact with that link, even if they have paused on it.

Therefore, for keyboard, an explicit activation might be best. Without adding another keyboard shortcut, we can use the existing "Link Dialog" or the popup that replaces it.

Suggestion.

My personal preference right now would be to have a single popup dialog for links, activated with click or Ctrl+K, that contains the existing link editing controls with just a single "fetch preview" checkbox. It would be simple, easy to discover if you are creating link elements, and easily usable from the keyboard. Whilst we would be lacking some of the explanatory text that the mockup has, once the user uses the feature once (say, out of curiosity) it is fairly understandable. I think the main drawback of this is that it wouldn't allow the user to change the preference from within the popup.

How will this combine with the upcoming editing popup?

I updated the mock-ups after talking with Ryan and exploring other solutions.
At first, we should keep things separate so one easy solution we can adopt is adding a "Card preview link" icon users can click to trigger the popup.
With that, we won't have any intereference with the link editing and we can safely postpone that addition for later.

Doing so will also fix the keyboard navigation and a11y problems we're stumbling upon.
We can allow users to hide that preview card link from the prefs if they don't care about any visible suggestion.
Later on, we will integrate the preview link feature inside the edit link popup.

Consider a user that wants to ultimately selectively use this feature, i.e. they want to explicitly select this for some of their links. After a few times, all they want from the popup is to be able to click "Replace this link with a preview". The current mockup is a bit out of place for this usage, even though this user is going to see this popup more often than the user who selected to always show a preview.

Mh, I'm not sure I understand this, sorry.

Ryan, related to this area, should we make a plan for 102 to fix basic link editing for mouse users which is currently badly broken in TB 91 for many links?

Bug 1716397 - Double-click on various links fails to trigger link properties. For many basic cases, no in-place way of editing link location with mouse

On a related note, do we have a bug for the "upcoming editing popup" mentioned in Henry's comment 28?
So far in TB 91 composition windows, we are not even showing the link target in status bar on hover.
Bug 194263 - Display the link target in status bar when hovering links in composition window (as in message reader)

Flags: needinfo?(ryan)
Summary: support rich embedding (previews) of links in composed content (Twitter Card, Open Graph data) → Support rich embedding (previews) of links in composed content (Twitter Card, Open Graph data)
Severity: normal → N/A

(In reply to Thomas D. (:thomas8) from comment #30)

Ryan, related to this area, should we make a plan for 102 to fix basic link editing for mouse users which is currently badly broken in TB 91 for many links?

Bug 1716397 - Double-click on various links fails to trigger link properties. For many basic cases, no in-place way of editing link location with mouse

On a related note, do we have a bug for the "upcoming editing popup" mentioned in Henry's comment 28?
So far in TB 91 composition windows, we are not even showing the link target in status bar on hover.
Bug 194263 - Display the link target in status bar when hovering links in composition window (as in message reader)

Not related for the scope of this bug, as we discussed - but we should be looking at link handling in general (not sure if we'll get them all this release).

Flags: needinfo?(ryan)

The following user flow (you can access it via the link) has been tested with users and works (very positive feedback and easy to discover/use). This feature should be implemented to match the user flow:

https://app.presentator.io/#/vgfdzj34/prototypes/31343/screens/212037?mode=preview

I was thinking about this new feature and how it is quite different from anything else currently supported by the editor. Will the xul:editor need updating to support this? In particular, I have a few open questions:

  1. What can we do to ensure the "card" is treated as an "atomic" element by the editor similar to a single <img> element?
    a. Making sure the element is a single caret stop point. I'm not sure if "caret stop point" is the right word, but I basically mean that the element would be treated as if it was a single character.
    b. Making sure the element has an appropriate accessible name for when the caret stops on it during editing.
    c. Preventing the user from typing different text within the card.
    d. Preventing the user from changing the font styling of the card.
    e. If you press backspace or delete, it removes the whole card.
    f. Dragging part of the card drags the whole card.
    g. Preventing the user from dragging out individual text or image parts of the card.
    h. Can you move the card to be "inline", or is it always "block"? If it is "block" only, do we prevent dragging it into the middle of a paragraph? If it is allowed to be "inline" how do we ensure that the text of the card does not inherit styling from ancestor inline element (e.g. <font>, <b>, <code>, etc) or block elements (e.g. <h1>, <address>, <pre>)?
    i. Prevent the user from converting the <a> element within the created card into a "sub-card".
  2. How do we handle the fact that the "link to card" conversion is non-instantaneous (and in principle could hang and timeout)?
    a. Whilst we are fetching the preview details, should we show a loading icon?
    b. Whilst we are fetching the preview details, should we "lock" the editor to prevent editing? If we don't lock the editor, then what would we do if the user deletes the link, moves the link, edits the link, presses Ctrl+Z or performs some other editing operation?
  3. Should we make the "link to card" conversion a single atomic editing operation? If you convert a link to a card, and then press Ctrl+Z, will this reverse the entire conversion? Or will you have to press Ctrl+Z twice: once to remove the card, and another to re-introduce the original link?

(In reply to Alessandro Castellani [:aleca] from comment #29)

[...] we can adopt is adding a "Card preview link" icon users can click to trigger the popup. [...] Doing so will also fix the keyboard navigation and a11y problems we're stumbling upon.

OK, this seems better than using hover. Would the icon be a "caret stop point", or something added with CSS ::after? If you can move over it with the caret, then you'd need to prevent the user from deleting it. Will it be possible to activate the popup with a keyboard?

Consider a user that wants to ultimately selectively use this feature, i.e. they want to explicitly select this for some of their links. After a few times, all they want from the popup is to be able to click "Replace this link with a preview". The current mockup is a bit out of place for this usage, even though this user is going to see this popup more often than the user who selected to always show a preview.

Mh, I'm not sure I understand this, sorry.

Never mind, it was a bit of a contrived and minor point. I just meant that the wording and presentation of the popup is focused on the first few times the feature is used, but the "Thunderbird can convert this URL into an embedded Preview card" message and the "Automatically add..." checkbox become unnecessary for users that regularly but selectively use the feature. But this isn't much of an issue.

Attachment #9262624 - Attachment description: WIP: Bug 1572648 - OpenGraph + Twitter Card support for message compose. r=aleca → Bug 1572648 - OpenGraph + Twitter Card support for message compose. r=aleca

(In reply to Henry Wilkes [:henry] from comment #28)
I agree, there are many issues with having thins kind of inline ad pop up next to the link. (The current patch does implement it.)
It's not optimizing for how people reasonably would expect to use it. I'd expect, they either wanted the preview or not. If they didn't for a certain case, simply Ctrl-Z to undo fixes each such case. If people don't want to use it at all, they are free to turn it off.
Now we add the rather cumbersome to use ad next to the link, but there' no saying if the link will be able to produce a reasonable preview. When we just automatically add or not, depending on the data we got, this is not a problem.

(In reply to Magnus Melin [:mkmelin] from comment #34)

(In reply to Henry Wilkes [:henry] from comment #28)
I agree, there are many issues with having thins kind of inline ad pop up next to the link. (The current patch does implement it.)
It's not optimizing for how people reasonably would expect to use it. I'd expect, they either wanted the preview or not. If they didn't for a certain case, simply Ctrl-Z to undo fixes each such case. If people don't want to use it at all, they are free to turn it off.
Now we add the rather cumbersome to use ad next to the link, but there' no saying if the link will be able to produce a reasonable preview. When we just automatically add or not, depending on the data we got, this is not a problem.

We got a lot of bad feedback in user testing around automatically doing this because it was confusing. What is this box and why has this happened? The icon provides discoverability versus just a hover or a click on the link and was clicked on out of curiosity in testing and then folks were pleasantly surprised by the feature. Perhaps there is an alternative which would be to pop up the dialog (asking to convert the link) upon pasting the link into the body of the message. This is not something we tested, but something that I would be open to testing if there are truly hard to resolve issues with implementing the icon. Otherwise, we should go with the UX that is supported by our user testing.

There are not issues with implementing it, the patch already does it (but need some css refinement).

It's a feature that's very common elsewhere: facebook, twitter, slack and others. So I don't know why that would be confusing. Everyone just does it unprompted, for good reason IMHO.
I would suggest doing some A/B testing with and without such a prompt, and see how that turns out now that we have it in action.

I'm all for A/B testing, but I disagree that doing something like that unprompted is a good solution, for us.

The majority of the platforms that do this by default are social media platforms, where this kind of behavior is common and expected.
We're an email client focused on privacy and user data control, and our users came to expect this sort of approach.

I understand the argument that "if you're pasting a link, you already know that link, so fetching that data is not a big deal or an issue", and I partially agree with that, but I also know for sure that the majority of our users will not agree with this and will see it as a terrible default and unwarranted usage of data, as well as a violation of privacy wince we're "reaching out" to exterior links without asking permission.
This might very much be a wrong perception and an incorrect assumption from our users, but it's gonna happen nonetheless and we need to mitigate this.

Showing a simple "indicator" icon as a pseudo element for links will allow us to expose this feature without disrupting workflow or forcing adoption.
Let's also be aware that this feature is very much a "visual-centric" feature, so visually impaired user will most likely never use it and will never actually need to use it.
Having an indicator exclusively reachable via mouse (and not keyboard), is a good first trial for an implementation that will benefit some and ignored by others.

In that "Create a preview card" popup, we should also have an option that allows users to "never show this indicator again", so users that don't care about it can completely ignore it and not have any visual prompt.
That's a very trivial implementation and we can easily support it.

Keywords: leave-open
Target Milestone: --- → 102 Branch
Pushed by mkmelin@iki.fi: https://hg.mozilla.org/comm-central/rev/28f4b7c207ff OpenGraph + Twitter Card support for message compose. r=aleca

Before the patch we always returned early from onPasteOrDrop if the content type was not "text/html", but this was removed https://hg.mozilla.org/comm-central/rev/28f4b7c207ff#l2.175

Just like that, I would have thought we'd still need to break early in this case around here https://hg.mozilla.org/comm-central/rev/28f4b7c207ff#l2.216 before we extract the text/html data.

  • Fix early return
  • Remove written out link from the card content, as we have the link above anyway
  • Create card also from standard document properties (title, meta description, link icon).
  • For larger open graph images, use large image preview.
  • Improve the css for the close button, especially for when it's over a dark image.

There's a bug where pasting anywhere in the compose window, such as the subject line, can cause a card to spawn in the message. E.g. paste "https://www.example.org" into the subject line will cause the text to appear in the message body instead. This is because onPasteDrop is attached to the compose window's document rather than the xul:editor.

Also, are we sure we want to be setting the subject of the message if it wasn't already set? https://searchfox.org/comm-central/rev/cbbeb29724f7e9e7f2fafe571a24a5b82c79b974/mail/components/compose/content/MsgComposeCommands.js#3761 I found the behaviour unexpected and I can't find a discussion for its motivation.

Also, are we sure we want to be setting the subject of the message if it wasn't already set

I think autofilled subject is helpful, and easy to remove if one doesn't want it. A prime use case is probably to just send someone a link to look at. This way you don't have to type in a subject, which people in general don't like too much.

I think opt out preferences for things like auto filled subject and even these previews are essential.

I am tired of listening to users complain about not being able to turn off the shiny new feature then are not interested in. Likening Mozilla to Microsoft and other multinationals that don't listen.

(In reply to Magnus Melin [:mkmelin] from comment #40)

Created attachment 9276036 [details]
Bug 1572648 - improve link previews. r=henry

  • Remove written out link from the card content, as we have the link above anyway

We should keep the link in the card. I don't think it makes sense to have the link above (and it undermines the beauty of this feature and puts it out of whack with where this is implemented elsewhere on the web). I think the link at the bottom of the card is an elegant way to display it and I like that it shortens excessively long urls (which can be revealed in its entirety down in the status bar upon hover).

Long story short, let's not keep the link above. It undermines the feature.

(In reply to Matt from comment #43)

I think opt out preferences for things like auto filled subject and even these previews are essential.

I am tired of listening to users complain about not being able to turn off the shiny new feature then are not interested in. Likening Mozilla to Microsoft and other multinationals that don't listen.

Currently, as it is planned to be implemented, it would be an opt-in feature with the ability to not show it again.

(In reply to Henry Wilkes [:henry] from comment #41)

Also, are we sure we want to be setting the subject of the message if it wasn't already set? https://searchfox.org/comm-central/rev/cbbeb29724f7e9e7f2fafe571a24a5b82c79b974/mail/components/compose/content/MsgComposeCommands.js#3761 I found the behaviour unexpected and I can't find a discussion for its motivation.

I also found this behavior unexpected. I can understand the motivation for it to an extent, but I don't think this is what we want to do by default.

Attachment #9276036 - Attachment description: Bug 1572648 - improve link previews. r=henry → Bug 1572648 - Improve link previews. r=henry

The points I raised about xul:editor in comment 33 are showing up in the current implementation. The preview card is not handled atomically by the editor, which makes it is very easy to mess around with it, and the cards get in the way of moving the text cursor. Also, the cards are fetched with some delay, which can cause problems if you don't wait around for it before moving the text cursor or editing the message. How can this be addressed, and will it be possible before release?

(In reply to Henry Wilkes [:henry] from comment #47)

The points I raised about xul:editor in comment 33 are showing up

The delay is/was a problem, which is one of the main reasons I had added the link directly (on top of the preview) in the patch. This way the delay wouldn't be disturbing. The other reason was to enable proper undo.
I've solved this in a another way with the current patch now: I add the link, select it and paste the preview above. This way there is no delay for inserting the actual link, and hitting undo will get you back to a (then unfortunately selected) normal link.

I don't think there is anything we can do do to make the card all atomic, more than it is now. Some users may want to edit it partly as well. I see little reason to prevent them from doing so.

The card is a block level element. I doubt it would ever look good inline with the text.

Comment on attachment 9276036 [details]
Bug 1572648 - Improve link previews. r=henry

[Approval Request Comment]
User impact if declined: buggy user experience for link previews
Testing completed (on c-c, etc.): c-c
Risk to taking this patch (and alternatives if risky): safe

Attachment #9276036 - Flags: approval-comm-beta?

Comment on attachment 9276036 [details]
Bug 1572648 - Improve link previews. r=henry

[Triage Comment]
Approved for beta

Attachment #9276036 - Flags: approval-comm-beta? → approval-comm-beta+
Depends on: 1773202

Let's close this not to confuse uplifts. Filed bug 1773202 to finish off the inline controls.
Comment 49 needs beta uplift.

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Flags: needinfo?(rob)
Resolution: --- → FIXED
Flags: needinfo?(rob)
Regressions: 1775208

I know of a few ways to cause a mess in the editor using this, strange styling or behaviours, or ways to throw a console error. So I don't think the current state is complete and polished enough for general usage. So to avoid bug reports that we would have to fix for esr, should we hide the "mail.compose.add_link_preview" preference for the 102 esr release? I know the preference is false by default, but it is currently exposed in the Composition settings.

Flags: needinfo?(ryan)
Regressions: 1777421
Flags: needinfo?(ryan)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: