Port bug 1372694 part 1 to TB: Stop making the default theme a heavyweight theme

RESOLVED FIXED in Thunderbird 61.0

Status

enhancement
RESOLVED FIXED
Last year
Last year

People

(Reporter: Paenglab, Assigned: Paenglab)

Tracking

unspecified
Thunderbird 61.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Bug 1372694 plans to remove the heavy weight theme support. The default theme is now a heavy weight too. They change this in bug 1372694. We need to follow to not end in a not working TB.
Posted patch No-HW-default-theme.patch (obsolete) — Splinter Review
I tried this together with the patch from bug 1372694 and it worked. :-)

Jörg, bug 1372694 has r+ on both patches and it can land soon. If Philipp hasn't reviewed it, when the m-c patches land, can you do it to not break TB?

Philipp, please can you check if all is correct for the build? This will affect Lightning too as the extension ID {972ce4c6-7e08-4474-a285-3208198ce6fd} will be removed and Lightning has a lot of references to it.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8970287 - Flags: review?(philipp)
Attachment #8970287 - Flags: review?(jorgk)
I'll look at this when bug 1372694 goes to inbound. You meant to say "... together with the patch*es* from bug 1372694 ...", right?
This works locally but on the server, the packaging fails with:

[task 2018-04-24T07:20:11.928Z] 07:20:11     INFO -  package> Error: /builds/worker/workspace/build/src/comm/mail/installer/package-manifest.in:161: Missing file(s): Thunderbird DailyDebug.app/Contents/Resources/defaults/profile/mimeTypes.rdf

That's because you remove that file here:
https://hg.mozilla.org/try-comm-central/rev/d15c8e51bc0fd1ede7525da549ceadc0f28773dd#l4.11

We did something related to mimetypes in bug 1446313 but never followed up removing that file completely from C-C. I filed bug 1456405 for that.

Let's try again removing that file from packaging as well:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3f9d4fe7a0ba43f1cc90ea67014a71839ab2e33b
(In reply to Jorg K (GMT+1) from comment #4)

> Let's try again removing that file from packaging as well:
> https://treeherder.mozilla.org/#/jobs?repo=try-comm-
> central&revision=3f9d4fe7a0ba43f1cc90ea67014a71839ab2e33b

If this doesn't work, I have a patch that doesn't removes it.
This includes the removal of mimeTypes.rdf from the package manifest. Further clean-up in bug 1456405.

Sadly there is a test failure:
mozmill/content-tabs/test-lwthemes.js | test-lwthemes.js::test_lightweight_themes_install

INFO -  SUMMARY-UNEXPECTED-FAIL | test-lwthemes.js | test-lwthemes.js::test_lightweight_themes_install
INFO -    EXCEPTION: Lightweight theme selected when there should not have been.
INFO -      at: test-lwthemes.js line 130
INFO -         test_lightweight_themes_install test-lwthemes.js:130 11

It looks like the test asserts that the initial theme is not an LW theme, but since heavy themes are gone, even the initial theme is LW. I'll fix it.
Attachment #8970287 - Attachment is obsolete: true
Attachment #8970287 - Flags: review?(philipp)
Attachment #8970287 - Flags: review?(jorgk)
Attachment #8970482 - Flags: review+
Sorry, my fix attempt of

 function test_lightweight_themes_install() {
-  // Before we run the test, check we've not got a theme already installed.
-  if (currentLwTheme())
-    throw new Error("Lightweight theme selected when there should not have been.");
-

only solved the first failure, there was another afterwards:

  EXCEPTION: Lightweight theme installation was not undone
    at: test-lwthemes.js line 91
       install_theme test-lwthemes.js:91 11
       test_lightweight_themes_install test-lwthemes.js:131 3

Too many tests that no LW theme is installed :-(

I'm running out of time now, so we will have to live with the test failure for now.
Keywords: leave-open
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/7011cd9d8481
Port bug 1372694 part 1 to TB: Stop making the default theme a heavyweight theme. r=jorgk
Posted patch 1456225-fix-test.patch (obsolete) — Splinter Review
I got the test going, but three minutes too late :-(
Attachment #8970486 - Flags: review?(acelists)
Comment on attachment 8970486 [details] [diff] [review]
1456225-fix-test.patch

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

Does the theme have a name that we could check for to see if the theme in the test replaced it?
Comment on attachment 8970482 [details] [diff] [review]
No-HW-default-theme.patch

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

I'm not sure why the mimetypes.rdf is touched in this patch. It could have been made in the appropriate bug.
Attachment #8970482 - Flags: review+
(In reply to :aceman from comment #10)
> Comment on attachment 8970486 [details] [diff] [review]
> 1456225-fix-test.patch
> 
> Review of attachment 8970486 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Does the theme have a name that we could check for to see if the theme in
> the test replaced it?

There is a "default-theme@mozilla.org". Maybe you can check this.
(In reply to :aceman from comment #11)
> I'm not sure why the mimetypes.rdf is touched in this patch. It could have
> been made in the appropriate bug.
Yes, I thought mail/app/profile/moz.build was removed entirely, but that's not the case:
https://hg.mozilla.org/comm-central/rev/7011cd9d8481#l4.2

Well, mistakes happen when you have 10 minutes to avoid bustage and get the Daily to run. Anyway, the removal is correct, it just happened in the wrong bug.
This works for me. I also enabled the skipped test_lightweight_themes_install_and_undo test function, it seems to pass for me now (maybe switching LW themes around is more reliable than from normal theme to LW one).
It works for me on Linux, try run here: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a176802b4c68b46bfdcd6693ac4e285ef2f6f055
Attachment #8970725 - Flags: review?(jorgk)
Attachment #8970725 - Flags: feedback?(richard.marti)
Attachment #8970486 - Attachment is obsolete: true
Attachment #8970486 - Flags: review?(acelists)
Comment on attachment 8970725 [details] [diff] [review]
1456225.patch fix test v2

Looks good (incl. boy-scout clean-up) and works for me on Windows. Too late for feedback and I'm not sure I'll wait for the try either. I cancelled the Windows one.

THANKS!
Attachment #8970725 - Flags: review?(jorgk)
Attachment #8970725 - Flags: review+
Attachment #8970725 - Flags: feedback?(richard.marti)
Keywords: leave-open
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/11a2c23125c1
adjust test-lwthemes.js for the default theme being lightweight too. r=jorgk
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 61.0
You need to log in before you can comment on or make changes to this bug.