1582941 - TEST-UNEXPECTED-FAIL | [snip]/mozmill/pref-window/test-font-chooser.js (port bug 1553804)

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1569065556/build/tests/mozmill/pref-window/test-font-chooser.js | test-font-chooser.js::test_font_name_displayed
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1569065556/build/tests/mozmill/pref-window/test-font-chooser.js | test-font-chooser.js::test_font_name_not_present
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1569065556/build/tests/mozmill/preferences/testCategoryColors.js | testCategoryColors.js::testCategoryColors

Some changes related to preferences:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c221a75fbf2957b6254fedb96436b5efbf&tochange=5ac55079c87e4cd25eafafa8662594b4a2

Aceman, that looks like some weekend workout :-(

Once I get my local build past bug 1582937 (with a backout), I'll take a look.

From the range, it could be:
Zibi Braniecki — Bug 1435915 - Remove preferences.properties. r=fluent-reviewers,Gijs,flod

Richard, do we need to restore something there? Even if you don't run tests, you can try the font chooser.

Comment 1

[Jorg K (CEST = GMT+2)]

Yes, we use preferences.properties
https://searchfox.org/comm-central/search?q=moz.*preferences.properties&case=false&regexp=true&path=
If they are gone, there won't be much fun.

Comment 2

[Jorg K (CEST = GMT+2)]

Looks like we need to port:
https://hg.mozilla.org/mozilla-central/rev/33b58a35cd4e#l3.11
https://hg.mozilla.org/mozilla-central/rev/33b58a35cd4e#l3.29
https://hg.mozilla.org/mozilla-central/rev/33b58a35cd4e#l4.12

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: 1582941-master-pw-string.patch - WIP

Hmm, that ports what I said, but I now see:

console.warn: "[fluent] Missing translations in en-US: forms-master-pw-fips-title, forms-master-pw-fips-desc."

Somehow we need to haul in the Fluent stuff.

Also I get:

Chrome file doesn't exist: C:\mozilla-source\comm-central\obj-x86_64-pc-mingw32
dist\bin\chrome\messenger\content\messenger\preferences\security.js
Failed to load file:///C:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/dist
/bin/chrome/messenger/content/messenger/preferences/security.js

and when clicking the advanced button for the fonts:

JavaScript error: chrome://messenger/content/preferences/subdialogs.js, line 108
: NS_ERROR_ILLEGAL_VALUE

There's a bunch of stuff broken here :-(

Comment 4

[Jorg K (CEST = GMT+2)]

Attachment: 1582941-master-pw-string.patch - WIP v2

Looks we can combine M-C's Fluent pref strings and out old fashioned ones, so I've copied the strings for the time being.

Comment 5

[Jorg K (CEST = GMT+2)]

Attachment: 1582941-master-pw-string.patch - WIP v3

preferences/security.js really does't exist. Perhaps a left-over from the prefs reshuffle.

Removing the script leaves me with:
JavaScript error: chrome://messenger/content/preferences/subdialogs.js, line 108: NS_ERROR_ILLEGAL_VALUE

That's where the subdialog is opened. Other dialogues don't open either, like the Config Editor, but that's not covered by a test like the font chooser.

Comment 6

[Jorg K (CEST = GMT+2)]

OK, the second issue comes from
Kris Maglione — Bug 1553804: Part 1 - Don't allow opening content windows with chrome openers. r=nika

Console says:
[720, Main Thread] WARNING: Chrome windows may never have content windows as their openers.: file c:/mozilla-source/comm-central/toolkit/components/windowwatcher/nsWindowWatcher.cpp, line 744

And the new code is:
https://hg.mozilla.org/mozilla-central/rev/0a4107e9e2af35b7db7cee4d180d6af3b3515af4#l1.17

We open the window here:
https://searchfox.org/comm-central/rev/db83c81aa34745e2149efb001739b9830af1dd45/mail/components/preferences/subdialogs.js#108

But the equivalent M-C code isn't much different:
https://searchfox.org/mozilla-central/rev/e18057a9613ffda06dfd3640209ca234ed7dc37d/browser/components/preferences/in-content/subdialogs.js#104

Comment 7

[Jorg K (CEST = GMT+2)]

Comment on attachment 9094397 [details] [diff] [review]
1582941-master-pw-string.patch - WIP v3

OK, this patch is a port of bug 1435915 and should be OK to review.

Comment 8

[Jorg K (CEST = GMT+2)]

Found it: We need this:
https://hg.mozilla.org/integration/mozilla-inbound/rev/56c14ca1d9e89633d4e635d77fdf44e1cc6d75dc#l1.12

Comment 9

[Jorg K (CEST = GMT+2)]

OK, this is a bit messy now since I'm mixing two ports here, but I don't feel like opening yet another bug on a Sunday.

Comment 10

[Jorg K (CEST = GMT+2)]

Even messier, as bug 1435915 just got backed out.

Comment 11

[Jorg K (CEST = GMT+2)]

Attachment: 1582941-chrome-windows.patch

Straight port of
https://hg.mozilla.org/integration/mozilla-inbound/rev/56c14ca1d9e89633d4e635d77fdf44e1cc6d75dc#l1.12

Comment 12

[:aceman]

Comment on attachment 9094400 [details] [diff] [review]
1582941-chrome-windows.patch

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

So this adds chrome=no to the dialog arguments?
This makes the dialog open, but I see some things are missing, like the pickers for font sizes and also below the "fixed width" font row there is a lone label "Minimum" on the right with missing size picker again.

Is the the whole patch? You mentioned other parts like adding Fluent strings in the previous comments.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/92eb7cad1116
Port bug 1553804: Add chrome=no when opening preferences subdialogs. rs=bustage-fix DONTBUILD

Comment 14

[Jorg K (CEST = GMT+2)]

Yes, that was the whole patch. I landed that now since the font chooser looks perfectly healthy to me. I've also run the two tests mentioned in comment #0 locally.

Have you pulled M-C after 3:40:08 PM when bug 1435915 got backed out? You need bug 1582961 if you haven't pulled. Or what else are you missing?

I'm going to retrigger a Daily since not being able to open subdialogues isn't so much fun.

Comment 15

[:aceman]

Comment on attachment 9094400 [details] [diff] [review]
1582941-chrome-windows.patch

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

Sorry, the size pickers were pushed to the right outside the visible area of the dialog due to a long named font. The dialog can be resized and then I see everything.
So all looks fine now, thanks.