1895599 - Omnijars cannot be read by the crash reporter to get localization information

Status: VERIFIED FIXED

(Toolkit :: Crash Reporting, defect, P1)

Description

[Alex Franchuk [:afranchuk]]

I had intended to explicitly test this behavior prior to merge, but in the many revisions and changes it slipped by. I have found that the zip crate is unable to read our omnijar files due to the optimized layout.

This is a significant defect that should be fixed before the crash reporter gets to release.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

We've already built the Fx126 RC and it's due to ship next Tuesday the 14th, FWIW.

Comment 2

[Donal Meehan [:dmeehan]]

We are in RC but we have already planned a 126.0 RC2. This goes to build on 2024-05-09.
If the fix for this can land today/tomorrow, and get an approval-mozilla-release uplift request tomorrow, then we can aim to take it in RC2.

Comment 3

[Alex Franchuk [:afranchuk]]

Attachment: Bug 1895599 - Fix omnijar reading with the zip crate. r=gsvelto

This crate is only used by the crash reporter and geckodriver, and
geckodriver is only used in testing (if I understand correctly).

In the future we will upstream changes to the zip crate which more
gracefully handles the case which they are trying to cover.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6153d392c944
Fix omnijar reading with the zip crate. r=gsvelto,supply-chain-reviewers

Comment 5

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7648e28b794b
Backed out changeset 6153d392c944 for causing build bustages CLOSED TREE

Comment 6

[Aron Cseh]

Backed out for causing build bustages

• Backout link
• Push with failures
• Failure Log

Comment 7

[Alex Franchuk [:afranchuk]]

Shoot, missed the warnings from the external-becoming-internal transition.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/deb3eaac74f4
Fix omnijar reading with the zip crate. r=gsvelto,supply-chain-reviewers

Comment 9

[Tracy Walker [:tracy]]

Andrei, please ensure we get a smoke test for this added to the Crash Reporter test suite.

Comment 10

[Alex Franchuk [:afranchuk]]

This can be tested by navigating to about:crashparent on a non-en-US locale installation of firefox. The crashreporter should have strings in the installation locale.

Alternatively, you can directly open the crashreporter binary in the installation. Doing so will show an error dialog that the tool is not made for being run by the user, however that error dialog should have localized strings.

Comment 11

[Donal Meehan [:dmeehan]]

:afranchuk could you preemptively add a release uplift request on this when you're ready?
Asking now as there is a timing factor since we are in RC week.

Of course, we won't take it until it's on central and when we uplift will be informed by the risk.

Comment 12

[Alex Franchuk [:afranchuk]]

Attachment: Bug 1895599 - Fix omnijar reading with the zip crate.

This crate is only used by the crash reporter and geckodriver, and
geckodriver is only used in testing (if I understand correctly).

In the future we will upstream changes to the zip crate which more
gracefully handles the case which they are trying to cover.

Original Revision: https://phabricator.services.mozilla.com/D209752

Comment 13

[Phabricator Automation]

release Uplift Approval Request

• User impact if declined: The crash reporter will not be localized to anything other than en-US (the fallback).
• Code covered by automated testing: no
• Fix verified in Nightly: yes
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: Navigate a non-en-US firefox to about:crashparent. The process will crash and the crash reporter will come up. Confirm text in the crash reporter is localized to the same locale as firefox.
• Risk associated with taking this patch: Low
• Explanation of risk level: One can confirm via Cargo.lock that the zip crate is only used by the crashreporter and geckodriver. Geckodriver is only used for testing and is not released. Regardless, the change in the zip crate should work for any typical zip files; it will only break reading files which have a zip archive appended after other arbitrary data, which isn't common.
• String changes made/needed: None
• Is Android affected?: no

Comment 14

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/deb3eaac74f4

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:afranchuk, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox126 to wontfix.

For more information, please visit BugBot documentation.

Comment 16

[Donal Meehan [:dmeehan]]

There is already a release uplift request

Comment 17

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/25d39c8fd1fb

Comment 18

[Ciprian Georgiu, Desktop QA]

We've rechecked this following the fixing of Bug 1895922, we can confirm that the issue has been resolved in the latest Nightly 127.0a1 and 126.0 RC2 on Windows 11 and Ubuntu 23.10.

However, the problem persists on macOS 13 and macOS 12, where the text within the crash dialog remains untranslated in localized builds tested (“fr”, “it”, “de”).

Donal suggested to file spearatly a bug for macOS, due to time constraints, and until we hear news from Alex. We filed as Bug 1896097.

Comment 19

[Ciprian Georgiu, Desktop QA]

Closing this verified as fixed since the localization problem was properly resolved and verified in Bug 1895922. The remainig issue on macOS is tracked separately under Bug 1896097.

(In reply to Tracy Walker [:tracy] from comment #9)

Andrei, please ensure we get a smoke test for this added to the Crash Reporter test suite.

We've created a smoke test to cover this scenario, which has been added to the crash reporting test suite, here.