Mini (Inline) Onebox Support RFC

rfc

(Neil Lalonde) #2

If we show the external-link-arrow glyph for these external links, we should probably do it for all (not just whitelisted). The favicon idea is neat though.


(Eli the Bearded) #3

Replacing my <a href="url-goes-here">description about url</a> description about a url will break my in-line use of html links. The og:title (or whatever) is unlikely to fit in the sentence the same way. Am I misunderstanding this? Is it a proposal for adding a footer with that information? I look at that Wikipedia thing and think that it is just about the decoration around the links. That I’d have no issue with.


(Robin Ward) #4

If the url has a description it will not be replaced. This will only happen on links that don’t have descriptions, for example

<a href="https://eviltrout.com">https://eviltrout.com</a>

or

[https://eviltrout.com](https://eviltrout.com)

(David Taylor) #5

I think it would be good to have a way to ‘opt out’ of the mini-oneboxing, in the same way as we can opt out of normal oneboxing by putting a space before the link. That said, I can’t think of a tidy way that could be done inline, whitespace clearly won’t work…


(Sam Saffron) #6

Some suggestions:

  • Always prefer opengraph/etc to title cause stuff like this is bad:

https://www.yelp.com/biz/hummus-kitchen-new-york

<title>Hummus Kitchen - Reservations - 340 Photos &amp; 598 Reviews - Middle Eastern - Theater District - New York, NY - Phone Number - Menu - Yelp</title>
  • If title is too long always truncate and add elipsis at some sane length. (site setting maybe)

  • To opt out easily use this syntax: <https://meta.discourse.org/t/mini-inline-onebox-support-rfc/66400/5> https://meta.discourse.org/t/mini-inline-onebox-support-rfc/66400/5 , very easily detectable in our engine.

  • We got to protect the Mini Onebox from hammering servers… should only request first N bytes of the page.

I like the external link icon, keep in mind some people may want to style it themselves for self owned domains and so on, so a class with the domain name would be nice. I would definitely avoid re-hosting and downloading custom favicons per site in the initial implementation, a default font-awesome icon for off-site is fine for the default. (also consider adding it optionally for any external links, at a minimum add a class on every A for internal vs external)


(Jeff Atwood) #7

Why? This would be incredibly noisy from a visual standpoint… we already show click counts so now we’re showing a favicon on every link on top of that?

Also we kinda buried the lede here… we are only proposing this behavior when self-linking to your own Discourse instance, for v1 of this anyway.

So you’d only get this behavior when posting internal links to your own Discourse at least for v1.9. We could unlock “all links” behind a site setting perhaps for testing.


(Eli the Bearded) #8

Well, the whitelisted domain part didn’t seem so buried lede to me. Maybe overthinking a V1 implementation. Or maybe the buried lede is self-links come whitelisted, and everything else is additional configuration.

But taking Wikipedia for example, there are many sister sites that are part of the same project but have different domains. Wikiquote, Wiktionary, Wikisource, etc. I like the idea of CSS styling that could be different for each. Sam’s class per domain would do that nicely.

To avoid clutter mini-onebox links like this could use a leading icon, or suppress showing a visit count (not suppress keeping one).


(Robin Ward) #9

I’ve just deployed the first version of this that supports linking to topics. For example: inlineRegexp with the new Markdown-it

Tomorrow I’m going to look at the whitelisted title version.


(Robin Ward) #12

Okay, I’ve added whitelist based support for inline oneboxing via <title> or opengraph tags:

(I also fixed the emoji issue).


(Sam Saffron) #14

One issue I noticed was that “rebake” is not fixing up the titles if you rename the original.

Also, looks like it is not working in list items: eg: CommonMark testing started here! :vulcan_salute:

vs

  1. CommonMark testing started here! :sadpanda:

Mini oneboxing of a discourse-topic doesn’t show click counts
(Robin Ward) #15

The problem here is I cache lookups for 1 day.

I think there might be an option when rebaking to invalidate the cache so I will look at that (as well as the list case).


(Sam Saffron) #17

I wonder title is a better default than og:title?

Cause for example:

https://github.com/discourse/discourse/blob/master/README.md

Becomes:

discourse/discourse

But could be:

discourse/README.md at master · discourse/discourse

Even NYTimes ships with pretty crappy og:title when consumed as a pure title: eg: Technology - The New York Times has the og:title technology, which really sucks unless coupled with an NY times logo.

My call would be to prefer title for now, cause it seems more correct on more sites.

Additionally, I think we should add a site setting for “enable mini onebox on all sites” that way we can properly test this here.

Longer term maybe we should look at adding a favicon, or maybe allowing the onebox gem to handle custom titles and so on.


(Sam Saffron) #18

Per:

Added the setting enable inline onebox on all domains, which is enabled here.

Also, after trial and error I think title is a far better default, so I changed it. I also fixed some issues with resolution of inline oneboxes in lists.

Some example mini oneboxes

Personally I LOVE :heart: Mini Onebox for everything ™ … I think it is spectacular! And think that we should make it default in a few weeks.

In contrast I think this is spectacularly ugly :japanese_goblin:

- https://www.nytimes.com/2017/08/02/science/gene-editing-human-embryos.html?hp&action=click&pgtype=Homepage&clickSource=story-heading&module=first-column-region&region=top-news&WT.nav=top-news 

- https://arstechnica.com/tech-policy/2017/08/fbi-says-man-posed-as-anonymous-to-ddos-media-to-force-removal-of-stories/ 

- https://en.wikipedia.org/wiki/Casu_marzu

- https://github.com/discourse/discourse/commit/f6bc572fb8ded5fe1f5d762ef2536c0ea9d24318

Long term I would like to spruce up inline oneboxes so it possibly includes a favicon or some other additional decorations and maybe even move it to the onebox gem.


(Jeff Atwood) #19

This is fine, but I am strongly opposed to adding a favicon to every link. At best an optional option.


(Sam Saffron) #20

Sure this is all going to be optional, the advantage of moving this to the onebox library long term is that it can “normalize the text”

Some sites like - GitHub other’s | Ars Technica, normalizing all of this has tons of appeals to me. But its definitely a v2 thing.


(Sam Saffron) #21

Almost 2 weeks later I still feel that enable inline onebox on all domains is a far better default. People are using it here and enjoying it lots. The edge case of https://www.cnn.com is fixed. There is minor level of muttering about people that need to discover the < > trick but I don’t think this warrants not making it default.

Should I go ahead and make it default?


(Jeff Atwood) #22

I dunno there is a lot going on, would rather take some
time on this. We spent 2 months cleaning up after always on regular onebox.


(Sam Saffron) #23

OK, will chase this up in 2 months.


(Erlend Sogge Heggen) #24

In https://meta.discourse.org/t/editing-wiki-posts-at-same-time-as-someone-else/30741/10 I’ve got a mini onebox and an ordinary onebox, both pointing to different parts of the same topic. After posting I changed the title of the topic, so I rebuilt did a “Rebuild HTML” to fix the titles in the oneboxes.

The regular onebox got fixed, but the mini onebox remains the same.


(David Taylor) #25

Just came across a very minor issue: In a composer preview, the mini-onebox doesn’t kick in without a protocol specified:

But when I click save, it turns into a mini-onebox: