If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Get rid of moz-page-thumb protocol.

RESOLVED WONTFIX

Status

()

Firefox
New Tab Page
RESOLVED WONTFIX
3 years ago
3 years ago

People

(Reporter: tomasz, Assigned: tomasz)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
There's no reason in keeping it. PageThumbs.jsm abstracts the retrieval of thumbnail so we don't have to have any special protocol for that. Just use getThumbnailURL.
(Assignee)

Updated

3 years ago
Assignee: nobody → tkolodziejski
Blocks: 1059558
Status: NEW → ASSIGNED
IMO moz-page-thumb is a really nice API for consumers (e.g. extension developers) which makes it trivial to get thumbnails. Is there some performance benefit to removing it?
Yeah, it seems to show up in the profiler when Tom was measuring. That's the only reason we were looking at it. I think we should probably rather promote using a function, e.g. PageThumbs.getThumbnailURL(), to get the URL. That way we can easily change how that works in the future.

I btw thought we promoted using that function, not the scheme directly.
Not sure but I like how easy it is to just drop in an <img src="moz-page-thumb://…"> to get thumbnails. There is one extensions using it on AMO. I like this about moz-anno:favicon:… as well.
(Assignee)

Comment 4

3 years ago
This would be nice if it created the thumbnails for you but it does not. So to get the thumbnail you still have to capture it before you can use it. And so the above won't work.
We could fix that though :)

Also, if this is showing up as a performance issue for New Tab, we can switch New Tab to not use it but that doesn't require us to remove the protocol handler at the same time.
(In reply to Matthew N. [:MattN] from comment #5)
> We could fix that though :)

That was why the protocol initially existed but we decided against that. Having perf impact and thumbnails being created too easily didn't seem like a good idea. We need more control over when that's done.

> Also, if this is showing up as a performance issue for New Tab, we can
> switch New Tab to not use it but that doesn't require us to remove the
> protocol handler at the same time.

True, it doesn't. I would really like to get rid of it though if the only thing we're breaking is a single add-on. We have too much legacy code already, no?
(Assignee)

Comment 7

3 years ago
Created attachment 8498530 [details] [diff] [review]
remove-moz-thumb-protocol.patch

TART says that getting rid of it does not affect the performance at all (the difference is not noticeable). However it looks like we're just doing less work after this change so it may still be worth merging. Tim?

We can also still discuss whether or not remove the moz-page-thumb.
Flags: needinfo?(ttaubert)
Avi's and my measurements show the same results, i.e. it doesn't seem to affect performance at all - contrary to what the DevTools profiler was reporting. It does seem worthwhile to reduce code complexity but without any gain I don't really see the value in breaking even one add-on either. Let's just close this and thanks for investigating!
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Flags: needinfo?(ttaubert)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.