1273501 - Create a thumbnail from a screenshot of current state of a URL

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P1)

Description

[Fernando Campo (:fcampo)]

We want to use a URL thumbnail as an image for the Tile.

- Taken at Tile creation time.
- Done in server side.
- stored as DataURI in Firebase

- framescript, canvas, drawWindow, pageShot were all mentioned as possibilities for the service, no decision made yet.

Comment 1

[Ian Bicking (:ianbicking)]

The easy way to start is to use the metadata extracted by PageMetadata, and get an image from .previews – which is typically a og:image image, or <link rel="image_src">.  That property also falls back on all <img> tags, which is a not-very-good-but-maybe-ok-for-a-little-while substitute.

Mark: is there a reason not to temporarily use a screenshot service for alpha only?

Note Activity Stream does not plan to use thumbnails for their tiles (because they tend to be crowded).

Comment 2

[Mark Banner (:standard8)]

(In reply to Ian Bicking (:ianb) from comment #1)
> The easy way to start is to use the metadata extracted by PageMetadata, and
> get an image from .previews – which is typically a og:image image, or <link
> rel="image_src">.  That property also falls back on all <img> tags, which is
> a not-very-good-but-maybe-ok-for-a-little-while substitute.

We've already seen that the og:image & other things in metadata aren't really a good substitute.

> Mark: is there a reason not to temporarily use a screenshot service for
> alpha only?

I guess I don't feel that a screenshot/thumbnail is necessary for alpha. It doesn't feel important in working out the user flow.

I'd rather we just look at creating a thumbnail from a loaded page (assuming we're talking about an owner here). There's already code for drag and drop of tabs that does this, it shouldn't be too hard just to base something on that.

Comment 3

[GitHub Autolander Bot]

Attachment: [loop] chrafuse:bug-1273501-create-thumbnail-from-screenshot-of-current-state-of-a-url > mozilla:akita

Comment 4

[Chris Rafuse :crafuse]

Comment on attachment 8761871 [details] [review]
[loop] chrafuse:bug-1273501-create-thumbnail-from-screenshot-of-current-state-of-a-url > mozilla:akita

Added the google screenshot to request and response.  TODO - need to wire up the data correctly in toc.jsx.  See PR comment.  Request hand-off to Manu to complete this.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

To add some more info, after playing around with Embed.ly, we had various problems with it: it regularly ended up returning robot-Amazon robot-detector CAPTCHAs.  Searching found a few other folks had run into this, and Embed.ly just suggested filing a ticket.  We also tried using Embedly with ebay (as a similar script), and found the images it produced wanting.

As a result, we decided to take a run at the thing @mancas originally implemented in his prototype: use google page speed for the screenshots.  So far, it seems to be working nicely.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Link to Github pull-request: https://github.com/mozilla/loop/pull/495

Obsoletes the previous PR.  If anyone in the EU timezones wants to pick this up and run with it, great.  If not, we'll keep pushing in the US tomorrow...

See the commentary in the PR for more details about where things are at,

Comment 7

[Fernando Campo (:fcampo)]

Comment on attachment 8761925 [details] [review]
Link to Github pull-request: https://github.com/mozilla/loop/pull/495

Timezoned review!

It works nicely indeed, and existing tests should suffice. Just needs few comments added to the code for the future changes, as I understand it is still a temporary solution.

A few comments not related to the PR but to the 'Add a tile' steps:
- It might be a little confusing to have a grey 'Loading' text while scrapping the metadata, as it takes longer time now than using the embed.ly service
- Intro key does not activate the adding process when writing the URL
- If URL not valid, button does not change back to active, adding panel does not hide. Should be one or the other.

I don't think it is worth it to open specific bugs for this issues, as everything should change when a final UI is presented, and clearly not issues to be fixed on this PR, but thought it would be better to mention them.

Comment 8

[Manuel Casas Barrado [:mancas]]

Landed in akita: https://github.com/mozilla/loop/commit/279275b31a6d08966b9c2df17eb1771065a717dd