888507 - [fig] Thumbnails are busted when upgrading to new about:home

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Wesley Johnston (:wesj)]

The new about:home uses a different aspect ratio for thumbnails than the one we have saved. We should scale the old ones to fit in the new area (from the upper left corner) so that we're showing something recognizable to the user. GeckoThumbnailView does this for us, but will have to be upgraded to not use its special scaling when other scale types are specified.

Comment 1

[Lucas Rocha (:lucasr)]

Sriram, is this going to be an issue? Did we change the aspect ratio of the thumbnails in the new about:home?

Comment 2

[Sriram Ramasubramanian [:sriram]]

Yes, we changed it. I am not sure what will happen to the old thumbnails though. Should we clear them up and re-build the cache?

Comment 3

[Lucas Rocha (:lucasr)]

Maybe the actionable items for this bug are:
- Understand how broken the old thumbnails will look after the upgrade
- Ensure the old thumbnails won't look completely broken (center the image and keep aspect ratio? fallback to favicon if thumbnail has old aspect ratio?)

ibarlow, what do you think?

Comment 4

[Ian Barlow (:ibarlow)]

I'd like to see an example of how broken it is. 

Also, the new thumbnails are shorter in height than the old ones, so I wonder if we can simply crop the bottoms off for the new awesomescreen UI. Otherwise, falling back to a favicon/coloured background until the user revisits the page seems like a reasonable solution.

Comment 5

[Wesley Johnston (:wesj)]

We have code for this exact situation in the tree (i.e. read comment 1). I'm not sure why we're just refusing to use it. I think we're under some belief that the performance impact of resizing images, even if they sometimes don't need resized, is too horrible for us to take. But since its OUR code, we can make it smart and fast.

We DO resize these images all the time, its just Android resizing them behind the scenes for us (using the exact same canvas.scale() code with a slightly different transform, that we use in ThumbnailView).

Comment 6

[Sriram Ramasubramanian [:sriram]]

We didn't want to use it back then because the thumbnail aspect ratio didn't change then. It was the same thumbnail size, hence didn't need any extra work. Now that we've changed the aspect ratio (within last 2 weeks), I'm fine with using the GeckoThumbnailView approach.

Comment 7

[Wesley Johnston (:wesj)]

Attachment: Screenshot

Example of the bustage. i.e. thumbnails are centered in the window, clipping off content at the top left, where the all important site logo probably is.

Comment 8

[Ian Barlow (:ibarlow)]

(In reply to Wesley Johnston (:wesj) from comment #7)
> Created attachment 794118 [details]
> Screenshot
> 
> Example of the bustage. i.e. thumbnails are centered in the window, clipping
> off content at the top left, where the all important site logo probably is.

Oh that's not as bad as I thought. Still though, is there a way to top-align them instead of centering?

Comment 9

[Sriram Ramasubramanian [:sriram]]

The current ThumbnailView doesn't respect the scaling type property. Do we want to do something about it? Should we create a new scaling type? Or override CENTER scaleType?

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

Is it true that this only happens right as you start using the new about:home design, but will "fix" itself as new thumbnails are generated?

If so, and given that it's not a critical issue, we could WONTFIX this bug and move on. I don't want to add hacks/tweaks for a temporary problem.

Comment 11

[Sriram Ramasubramanian [:sriram]]

Once the thumbnails are refreshed, this problem wouldn't exist. And for the small ratio change, there's nothing the user could tell that the screenshot is not aligned to the top.

Comment 12

[Wesley Johnston (:wesj)]

I'd live with WONTFX here. We're basically tying these particular thumbnails to the thumbnail generation code. If we ever change that code to generate thumbnails at a different aspect ratio, these will regress. I like removing that coupling and just making these look good regardless of what the thumbnail generation code looks does/returns, but I don't think this should block anything.

Comment 13

[Sriram Ramasubramanian [:sriram]]

As decided.