1665175 - Theme icons not displayed in multistage welcome

Status: VERIFIED FIXED

(Firefox :: Messaging System, defect, P1)

Description

[Punam Dahiya [:pdahiya]]

STR:

1. Build latest m-c
2. Open about:welcome
3. Navigate to theme screen by clicking 'start setup'

Expected

1. On theme screen themes icons shows inside respective tiles

Actual
Theme icons are not visible ( See attached) with error in console as "Security Error: Content at resource://activity-stream/aboutwelcome/aboutwelcome.css may not load or link to chrome://mozapps/content/extensions/default-theme.svg"

Comment 1

[Punam Dahiya [:pdahiya]]

This appears to be regression caused by bug 1145314. NI Chris to look at it and help with the fix.

Comment 2

[Punam Dahiya [:pdahiya]]

Attachment: Theme screen with missing icons

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

Hey thanks for reporting and sorry for breaking. FWIW, I pushed to Treeherder applying an additional patch which would throw an assertion if something in our TREE was allowed to load before and not after enforcing the stricter security checks. In turn, that means there is no automated test exercising that code path. Probably worth filing a follow up and adding one?

Anyway, when fixing Bug 1654258 we started loading other css files using chrome: but apparently not aboutwelcome.css which I think is what we should do there. I'll upload a patch in a minute.

Hopefully that does not introduce any other side effects on Treeherder.

If thinks are critical for testing you could flip the pref security.caps.allow_uri_is_ui_resource_in_checkloaduriflags until we have fixed the problem within this bug.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

Attachment: Bug 1665175: Load aboutwelcome.css using chrome:.

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

Ah, we need to list files separately in the jar.mn file:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a44cc57d2dd096a2340afaacf16cfe8a9073c01f

updated the patch as well.

Comment 7

[Punam Dahiya [:pdahiya]]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #3)

Hey thanks for reporting and sorry for breaking. FWIW, I pushed to Treeherder applying an additional patch which would throw an assertion if something in our TREE was allowed to load before and not after enforcing the stricter security checks. In turn, that means there is no automated test exercising that code path. Probably worth filing a follow up and adding one?

Curious, what automated test should look like. The underlying issue is stricter security checks that caused aboutwelcome.css failing to load image at chrome://mozapps/content/extensions/...svg, where as some chrome url images such as chrome://activity-stream/content/data/content/assets/short-zap.svg loads successfully.

Should the tests be checking for all chrome URL usage outside chrome://activity-stream/content/..??

Anyway, when fixing Bug 1654258 we started loading other css files using chrome: but apparently not aboutwelcome.css which I think is what we should do there. I'll upload a patch in a minute.

+1 , tried attached patch and it does fix the issue reported in bug

Comment 8

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Punam Dahiya [:pdahiya] from comment #7)

Curious, what automated test should look like. The underlying issue is stricter security checks that caused aboutwelcome.css failing to load image at chrome://mozapps/content/extensions/...svg, where as some chrome url images such as chrome://activity-stream/content/data/content/assets/short-zap.svg loads successfully.

It's not about the security check itself. As far as I can tell there is no test which loads any of the mozapps/content/extensions/...svg in any way - otherwise my assertion would have directed me in that direction.

Put differently, I guess we need a test for about:welcome which would click the start setup button.

Comment 9

[Punam Dahiya [:pdahiya]]

It's not about the security check itself. As far as I can tell there is no test which loads any of the mozapps/content/extensions/...svg in any way - otherwise my assertion would have directed me in that direction.

Put differently, I guess we need a test for about:welcome which would click the start setup button.

Sounds good updated tests in the followup bug 1665756, thanks!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5d6e247a8dde
Load aboutwelcome.css using chrome:. r=mconley,pdahiya

Comment 11

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/5d6e247a8dde

Comment 12

[Ciprian Muresan [:cmuresan], Ecosystem QA]

I have managed to reproduce this issue on the Nightly build from 2020-09-15 (BuildID 20200915154719) and have verified that the issue is no longer reproducible on the latest Nightly (83.0a1, 20200922154306) and Beta (82.0b2, 20200922183749) builds using Windows 10, macOS 10.15, and Linux MX 4.19.