Closed Bug 1787121 Opened 2 years ago Closed 2 years ago

[Localization] Private Browsing icon remains on Start menu after uninstall for localized builds.

Categories

(Firefox :: Private Browsing, defect)

Firefox 105
Desktop
Unspecified
defect

Tracking

()

RESOLVED FIXED
107 Branch
Tracking Status
firefox-esr102 --- unaffected
firefox105 --- wontfix
firefox106 --- wontfix
firefox107 --- verified

People

(Reporter: mchiorean, Assigned: bhearsum)

References

(Depends on 1 open bug)

Details

(Whiteboard: [fidedi-pbm])

Attachments

(6 files, 1 obsolete file)

Attached image screenshot1

Found in

  • Beta 105.0b2

Affected versions

  • Beta 105.0b2

Tested platforms

  • Affected platforms: Windows (7, 10).
  • Unaffected platforms: Ubuntu20.4, MacOs.

Steps to reproduce

  1. Uninstall all FF builds from desktop.
  2. Delete registry and App Data from PC.
  3. Perform a clean install of a localized build (I used fr build).
  4. Set prefs “browser.privacySegmentation.enabled” and “browser.privacySegmentation.windowSeparation.enabled” to true.
  5. Restart FF.
  6. Open a Private Window.
  7. Close and uninstall FF.
  8. Check programs in Start for FF icons.

Expected result

  • No FF icons should be displayed in Start->Programs.

Actual result

  • Private FF icon is displayed in Start->Programs after uninstall. If trying to open the remaining icon a screen if displayed (see second screenshot).

Regression range

  • Will add one asap.

Additional notes

  • Issue reproduces on French(.fr) and Spanish (.es-ES) localized builds but might affect all localizations.
  • If Private icon is pinned to Start Menu, icon is displayed on Start Menu after uninstall also.
  • Issue is not reproducing on EN.
Attached image Screenshot2
Severity: -- → S3
Depends on: 1750758
Assignee: nobody → bhearsum
Whiteboard: [fidedi-pbm]

I wasn't able to reproduce this in a clean environment. The most likely thing here is that the shortcuts log is not getting created or perhaps updated when the shortcut is created (when you first restart Firefox after setting those prefs). Could you paste the contents and permissions of any ini files in C:\ProgramData\Mozilla-1de4eec8-1241-4177-a864-e594e8d1fb38, please? That might give us some additional insight.

If there's something we can do to fix this case once we've identified it better I'd like to, but this will also be a very unlikely scenario in 106. The installer will be creating shortcuts beginning with 106, so there shouldn't be any real way for there to be issues with the shortcuts log so S3 seems like the right severity.

Flags: needinfo?(monica.chiorean)

I attached this file from "C:\ProgramData\Mozilla-1de4eec8-1241-4177-a864-e594e8d1fb38" before uninstall. Please let me know if it helps. Thank you.

Flags: needinfo?(monica.chiorean)

Thank you, that is helpful! I see that the shortcut is indeed listed there. My earlier test was with a localized build that didn't have any unicode characters in its shortcut name. Testing again with fr I was able to repro the issue.

I suspect we generally don't handle shortcuts with unicode characters in their filenames, and it simply hasn't been an issue up until now.

Turns out the issue is a bit more subtle: the shortcut logs we create at runtime (through nsWindowsShellService) are UTF-8 encoded, which NSIS is unable to read correctly. If I rewrite the file as as UTF-16 LE, it works fine. (The shortcuts log created by the installer is already encoded like this, likely because it's NSIS writing it.)

This is an extremely rough WIP. It does write UTF-16LE files on Windows, but something is going wrong still. Eg: "privée" ends up written as "privᅢᄅe". There's also other things that need fixing:

  • Needs to work on all platforms (I doubt this will compile anywhere but Windows)
  • General cleanup

I also wonder if maybe the better path to fix this is to avoid making nsINIParser know about encodings at all, and instead have it be able to return a formatted string to write, and let nsWindowsShellService do the file operations?

Has STR: --- → yes

This allows callers to re-encode the data (eg: to UTF-16) and write or otherwise do with the formatted data as they wish.

I ended up adding/exposing additional init methods in part because I thought it made sense to support initialization from a string since we can now write to one, but also to make testing easier.

It turns out that the INI parser used by NSIS only supports UTF-16LE encoding. Our INI parser doesn't support UTF-16 at all, so we need to a bit of work ourselves to write this file correctly.

Depends on D157377

Attachment #9291773 - Attachment is obsolete: true
Pushed by bhearsum@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a29ea236670d
add support for formatting INI files to nsINIParser r=xpcom-reviewers,kmag
https://hg.mozilla.org/integration/autoland/rev/4e30a2b996fb
write runtime shortcuts log in UTF-16LE encoding r=bytesized
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 107 Branch

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit auto_nag documentation.

Comment on attachment 9294762 [details]
Bug 1787121: add support for formatting INI files to nsINIParser r?#xpcom-reviewers!

Beta/Release Uplift Approval Request

  • User impact if declined: Users of localized builds that uninstall Firefox will not have their private browsing shortcuts removed
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: 1) Install a localized build of Firefox
  1. Run it, to make sure a Private Browsing shortcut is created
  2. Uninstall it

Expected result: the Private Browsing shortcut should be removed

  • List of other uplifts needed: None
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): Arguably this is low risk, but because this patch refactors some writing ini code that is used by other parts of Firefox (most notably the profile manager) the failure mode could be quite bad if something did go wrong.
  • String changes made/needed: no
  • Is Android affected?: No
Attachment #9294762 - Flags: approval-mozilla-beta?
Attachment #9294763 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Ben, is that a recent regression or a long standing issue? If the later, I would prefer to let it ride the train as it is set to S3 and I am not seeing duplicates that would indicate that this is impacting many users.

Flags: needinfo?(bhearsum)
QA Whiteboard: [qa-triaged]

(In reply to Pascal Chevrel:pascalc from comment #14)

Ben, is that a recent regression or a long standing issue? If the later, I would prefer to let it ride the train as it is set to S3 and I am not seeing duplicates that would indicate that this is impacting many users.

It's recent, but I think we can ship with it as it only affects uninstall. Adding Romain in case he has a differing opinion.

Flags: needinfo?(bhearsum)

I agree it can wait 107.

Attachment #9294762 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Attachment #9294763 - Flags: approval-mozilla-beta? → approval-mozilla-beta-

Could not reproduce issue on Win10/Win7 using FF build 107.0a1(20220930093536), used different locals (it, fr, es, zh-CN, ar).

QA Whiteboard: [qa-triaged]
Flags: qe-verify+
Flags: qe-verify+
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: