Closed Bug 888507 Opened 11 years ago Closed 11 years ago

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


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




Tracking Status
fennec 26+ ---


(Reporter: wesj, Assigned: sriram)




(1 file)

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.
Sriram, is this going to be an issue? Did we change the aspect ratio of the thumbnails in the new about:home?
Flags: needinfo?(sriram)
Priority: -- → P1
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?
Flags: needinfo?(sriram)
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?
Flags: needinfo?(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.
Flags: needinfo?(ibarlow)
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).
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.
tracking-fennec: --- → ?
OS: Linux → Android
Hardware: x86 → All
tracking-fennec: ? → 26+
Attached image 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.
(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?
Assignee: nobody → 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?
Flags: needinfo?(wjohnston)
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.
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.
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.
Flags: needinfo?(wjohnston)
As decided.
Closed: 11 years ago
Resolution: --- → WONTFIX
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.