1555340 - Abuse report does not display the strings for the buttons

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[CosminB]

Attachment: BugReport.gif

[Affected versions]:

• Firefox 69.0a1 (20190529065901)

[Affected platforms]:

• Win 7 64-bit
• Mac OS X 10.14.1

[Prerequisites]

• Set extensions.abuseReport.enabled to true
• Set extensions.htmlaboutaddons.enabled to true

[Steps to reproduce]:
1.Install an extension, for example https://addons.mozilla.org/en-US/firefox/addon/google-search-link-fix/?src=search
2.Navigate to about:addons and select “Extensions”
3.Click on the more options button.
4.Click on “Report” and observe the panel.

[Expected results]:

• The report is displayed correctly and contains all the strings for the button and radio buttons.

[Actual results]:

• The buttons don't have any string displayed.

Please see the attached video.

I did a regression window and observed that Bug 1546432 is the one that caused this regression.

Pushlog:https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d551d37b9ad0dd1c8ad2e87c74344f623fc4b694&tochange=c87317c4190283cc4352331417babef3d3f9546

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

A mozregression bisect that I executed locally pointed to D31554 as the first commit on which the abuse report frame's elements started to not be translated automatically.

From a quick look I gave it, it seems that the fluent locale files are being loaded and they are in use (e.g. removing the link element for the toolkit/about/abuseReports.ftl fluent file from abuse-report-frame.html triggers test failures when browser_html_abuse_report.js is executed, as expected), but the elements with the data-l10n-id attribute are not being translated automatically as it was happening before Bug 1546432.

I briefly verified that calling explicitly from the webconsole document.l10n.translateFragment(document.body) in the abuse report frame actually triggers the translation of all the DOM elements in the abuse report frame, but I haven't yet been able to investigate it further and find out the exact reasons why ("it seems that") DOMLocalization is not observing the abuse report frame's document for changes and apply the localization automatically.

Comment 2

[Axel Hecht [:Pike]]

Even just changing one data-l10n-id renders all strings.

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

I'll investigate it.

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

I was able to reproduce it, and am investigating.

The issue seems to be that for some reason, when the templates in [0] get turned into a fragment appended to the document [1] we fire all the right Mutations, but we can't get an nsPresContext so we silently return in [2].

[0] https://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/content/abuse-report-frame.html#17
[1] https://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/content/abuse-report-panel.js#488
[2] https://searchfox.org/mozilla-central/source/dom/l10n/Mutations.cpp#153

Obviously, we should at least warn if not assert if that happens, but then, I'm not sure why it happens in the first place.

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: missing_prescontext.diff

This patch turns it into a crash scenario in the debug build.

The STR with this patch is:

1. open about:config
2. Toggle to "true"

• extensions.abuseReport.enabled
• extensions.htmlaboutaddons.enabled

3. Open about:addons

Current result:
Crash in [0]

Expected result:

Refresh Driver gets started correctly and triggers element localization.

[0] https://searchfox.org/mozilla-central/source/dom/l10n/Mutations.cpp#153

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

smaug said that it may be that prescontext is not ready yet.

That would mean that the order is:

1. Right before layout trigger DocumentL10n::InitialTranslation
2. That collects elements, starts Mutations, and translates collected elements in that order
3. connectedCallback is called and triggers Mutations
4. But nsPresContext is not ready yet.

If that's the case, one thing we could do is translate all elements until we can start RefreshDriver as they come, rather than coalescing them into a single refresh. That would likely be suboptimal from the performance perspective, but maybe not significant.

I'm also not sure if that's what is going on, because it seems to me like the document is translating some mutations before that crash happens, so it also may be that somehow because its CE we get a wrong document?

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1555340 - Check for pending mutations when Document creates PresShell.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/808f1d8b4fb8
Check for pending mutations when Document creates PresShell. r=smaug

Comment 9

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/808f1d8b4fb8

Comment 10

[CosminB]

Attachment: Bug1555340.gif

This issue is verified as fixed on Firefox 69.0a1 (20190604034844) under Win 7 64-bit and Mac OS X 10.14.1.

Please see the attached video.