Closed Bug 1598903 Opened 3 months ago Closed 3 months ago

Port bug 1575905 Part 1: Show theme previews for built-in themes

Categories

(Thunderbird :: Theme, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 72.0

People

(Reporter: Paenglab, Assigned: Paenglab)

Details

Attachments

(1 file, 1 obsolete file)

Bug 1575905 adds a map in a const to specify the preview image in Add-ons manager.
We use other names for the light/dark theme and need somehow to add our theme names to this const to automatically show the preview.

We need this two themes to the map:
[
"thunderbird-compact-light@mozilla.org",
"chrome://mozapps/content/extensions/firefox-compact-light.svg",
],
[
"thunderbird-compact-dark@mozilla.org",
"chrome://mozapps/content/extensions/firefox-compact-dark.svg",
],

Geoff, is it somehow possible to add them to the const?

Flags: needinfo?(geoff)

Oh, that's annoying. The Map is totally inaccessible because it's const. It should be possible to swap out getScreenshotUrlForAddon the same way we did getAddonMessageInfo, and short-circuit it for those two add-ons.

Flags: needinfo?(geoff)

This patch shows the previews. I'm not sure if this is the right way with hijacking the function completely.
This is why I have set only f?. Should this be by accident the correct way don't hesitate to set r+ ;)

Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #9111373 - Flags: feedback?(geoff)
Comment on attachment 9111373 [details] [diff] [review]
1598903-addon-TB-theme-preview.patch

Review of attachment 9111373 [details] [diff] [review]:
-----------------------------------------------------------------

Yes this looks like it works but we can do it better.

::: mail/base/content/aboutAddonsExtra.js
@@ +64,5 @@
>  
> +  getScreenshotUrlForAddon = function(addon) {
> +    if (THUNDERBIRD_THEME_PREVIEWS.has(addon.id)) {
> +      return THUNDERBIRD_THEME_PREVIEWS.get(addon.id);
> +    }

You can stop after these four lines and just return the result of the original function.

First, save the original function to a variable: let _getScreenshotUrlForAddon = getScreenshotUrlForAddon;

Then in the new function: return _getScreenshotUrlForAddon(addon);
Attachment #9111373 - Flags: feedback?(geoff) → feedback+

Ah yes, this works :-)

Attachment #9111373 - Attachment is obsolete: true
Attachment #9111666 - Flags: review?(geoff)
Comment on attachment 9111666 [details] [diff] [review]
1598903-addon-TB-theme-preview.patch

That's it.
Attachment #9111666 - Flags: review?(geoff) → review+

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/83336f180281
Port bug 1575905 Part 1: Show theme previews for built-in themes. r=darktrojan

Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 72.0
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/0f56d3bf9300
Fix linting mistake; rs=me DONTBUILD
You need to log in before you can comment on or make changes to this bug.