1540445 - Port bug 1525762: Convert built-in LWTs to static themes

Status: RESOLVED FIXED

(Thunderbird :: Theme, task)

Description

[Jorg K (CEST = GMT+2)]

Bug 1525762 is now on inbound. We're leaving it to the last minute?

Comment 1

[Richard Marti (:Paenglab)]

Attachment: 1540445-LWT-to static-theme.patch

I hope I found all changes we need to change too.

Not tried locally as there are too many patches to import but started a try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1e398992cf79463037db7db9c54513d9e1968f2c

Comment 2

[Jorg K (CEST = GMT+2)]

You're an expert now with that M-C pinning ;-) - Looks like a lot of changes, thanks!

Comment 3

[Richard Marti (:Paenglab)]

Tried the try build and it seems to work. It looks like before. The only difference, there is a version in the add-on info.

Comment 4

[Jorg K (CEST = GMT+2)]

Ahh, you should have run the tests :-( - Here they are:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=7f06c2b5dbd90b06664f1afb48124dda99877bc3

Comment 5

[Jorg K (CEST = GMT+2)]

Comment on attachment 9054726 [details] [diff] [review]
1540445-LWT-to static-theme.patch

So most of this comes from Part 2a:
https://hg.mozilla.org/mozilla-central/rev/b62c3bde4bc3

The prefs from here, part 2b:
https://hg.mozilla.org/mozilla-central/rev/83612982ab33

And the hunk with `isCompactTheme` from part 3f and 6:
https://hg.mozilla.org/mozilla-central/rev/b9f524da2a61#l1.12
https://hg.mozilla.org/mozilla-central/rev/5d5860192199#l1.12

Nice exercise for the reviewer to retrace your actions ;-) - Looks like it's all ported properly, I can't tell whether anything was missed, but from looking through the various changes of the M-C bug, I saw lots of toolkit changes.

Comment 6

[Jorg K (CEST = GMT+2)]

OK, as expected, mozmill/content-tabs/test-lwthemes.js fails now. Aceman, could you please take a look at that one.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2ccd070aadad
Port bug 1525762: Convert built-in LWTs to static themes. r=jorgk
https://hg.mozilla.org/comm-central/rev/503354839d42
temporarily disable mozmill/content-tabs/test-lwthemes.js. rs=bustage-fix DONTBUILD

Comment 8

[Jorg K (CEST = GMT+2)]

Attachment: lwt-fix.patch WIP

This works a bit more, but still doesn't pass. We get:
msgMail3PaneWindow.js, line 1634: TypeError: this._manager.parseTheme is not a function
since parseTheme was removed in part 3e of the M-C bug:
https://hg.mozilla.org/mozilla-central/rev/b7481329e0b7#l3.152

So apart from changes to the test, there are changes necessary in msgMail3PaneWindow.js.

Comment 9

[Jorg K (CEST = GMT+2)]

Well, looks like LightWeightThemeWebInstaller was removed from M-C and events like InstallBrowserTheme don't exist any more either. But that already happened in bug 1525511, part 1:
https://hg.mozilla.org/mozilla-central/rev/e711fcd0a02d#l3.13.

More research: The LWT stuff in TB was landed here: https://bugzilla.mozilla.org/show_bug.cgi?id=538335#c40

I little hard to tell what needs to be done here.

Comment 10

[Jorg K (CEST = GMT+2)]

Hmm, the test prints

dispatching InstallBrowserTheme
dispatching PreviewBrowserTheme

https://searchfox.org/comm-central/rev/4fd2bb880f8af2c7979b691eea616f587a9f2d4b/mail/test/mozmill/content-tabs/html/test-lwthemes.html#30

but the listener is in msgMail3PaneWindow.js in LightWeightThemeWebInstaller whose sister was was removed from M-C and which doesn't work any more since the it gets the LightweightThemeManager and calls parseTheme which was removed.

So no idea what the ultimate aim is for the test.

Comment 11

[Jorg K (CEST = GMT+2)]

Attachment: lwt-fix.patch (v2)

Just remove it all?

Comment 12

[Geoff Lankow (:darktrojan)]

Comment on attachment 9054974 [details] [diff] [review]
lwt-fix.patch (v2)

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

This is from when we had the capability to roll over a theme on AMO/ATN and preview it. Looks like we won't be using this any more, which is a shame, I liked it. :-(

Comment 13

[Magnus Melin [:mkmelin]]

Comment on attachment 9054974 [details] [diff] [review]
lwt-fix.patch (v2)

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

Yep, let's remove

Comment 14

[Jorg K (CEST = GMT+2)]

I'll take this as r+ :-)

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/c1302806cfdc
Remove LightWeightThemeWebInstaller code from 3pane and remove test-lwthemes.js. r=darktrojan,mkmelin