[Win & Linux] Reinstalling an App that was previously launched deletes an old profile and re-creates a different app profile

VERIFIED FIXED in Firefox 16

Status

Firefox Graveyard
Web Apps
P1
major
VERIFIED FIXED
5 years ago
a year ago

People

(Reporter: jsmith, Assigned: marco)

Tracking

({dataloss})

unspecified
Firefox 16
dataloss

Details

(Whiteboard: [blocking-webrtdesktop1+], [qa!])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
Steps for Windows and Linux:

1. Go to apps.mozillalabs.com/appdir
2. Install Mozilla QA WebRT Tester
3. Launch the app
4. Go to firefox --> geolocation --> position.html
5. Select remember my choice and make a selection
6. Close the app
7. Reinstall the app
8. Launch the app
9. Go to firefox --> geolocation --> position.html

Expected:

The choice remembered should still be remembered and reused for geolocation.

Actual:

A prompt appears again asking for the user to share their location - the choice made originally that was remembered was lost.

Additional Notes:

Only appears to happen on Windows & Linux. Works correctly on Mac.
(Reporter)

Updated

5 years ago
Component: Web Apps → Webapp Runtime
QA Contact: webapps → webapp-runtime
(Assignee)

Comment 1

5 years ago
This is most probably caused by Firefox removing the profile directory on apps re-installation (only on Win and Linux).
(Reporter)

Comment 2

5 years ago
(In reply to Marco Castelluccio from comment #1)
> This is most probably caused by Firefox removing the profile directory on
> apps re-installation (only on Win and Linux).

Confirmed on Windows. The profile directory is indeed wiped on a reinstall of a web application previously launched.
(Reporter)

Updated

5 years ago
Summary: [Win & Linux] Reinstalling an App Where Geolocation Choice Was Remembered Loses Choice Remembered → [Win & Linux] Reinstalling an App that was previously launched deletes and re-creates a different app profile
(Reporter)

Updated

5 years ago
Summary: [Win & Linux] Reinstalling an App that was previously launched deletes and re-creates a different app profile → [Win & Linux] Reinstalling an App that was previously launched deletes an old profile and re-creates a different app profile
(Reporter)

Comment 3

5 years ago
k9o nomination - we shouldn't be wiping profile data on a reinstall - otherwise we lose anything that's tied to an app profile.
Severity: normal → major
blocking-kilimanjaro: --- → ?
(Assignee)

Updated

5 years ago
Assignee: nobody → mar.castelluccio
Status: NEW → ASSIGNED
(Reporter)

Updated

5 years ago
Keywords: dataloss
(Assignee)

Comment 4

5 years ago
Created attachment 633687 [details] [diff] [review]
Patch
Attachment #633687 - Flags: review?(felipc)
(Reporter)

Updated

5 years ago
Component: Webapp Runtime → Web Apps
QA Contact: webapp-runtime → webapps
(Assignee)

Comment 5

5 years ago
Created attachment 633793 [details] [diff] [review]
Patch v2

Why in _createConfigFiles (for Win) there are:
> writer = null;
> factory = null;
Do we need to explicitly set them to null?
Attachment #633687 - Attachment is obsolete: true
Attachment #633687 - Flags: review?(felipc)
Attachment #633793 - Flags: review?(felipc)
blocking-kilimanjaro: ? → +
Comment on attachment 633793 [details] [diff] [review]
Patch v2

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

r- for the for..of fix and hoping that we can do the refactor suggested, but everything looks fine

::: browser/modules/WebappsInstaller.jsm
@@ +264,5 @@
> +                           this.configJson,
> +                           this.webappINI
> +                          ];
> +
> +      for(let file in filesToRemove) {

this doesn't work, you need to use for..of instead of for..in to iterate through the array contents instead of the keys.

@@ +295,3 @@
>          this.startMenuShortcut.remove(false);
>        }
> +    } catch(ex) {}

I wasn't gonna request this in this bug, but since there's the for..of thing to fix, I believe we should do it too:

I think we can make this whole section a bit cleaner by doing:

```
let filesToRemove = [this.desktopShortcut, this.startMenuShortcut];

if (keepProfile) {
  filesToRemove.append(this.iconFile);
  filesToRemove.append(...);
} else {
  // Remove everything
  filesToRemove.append(this.installDir);
}

... for loop to remove files ...
```
Attachment #633793 - Flags: review?(felipc) → review-
(Reporter)

Updated

5 years ago
Priority: -- → P1
Whiteboard: [blocking-webrtdesktop1+]
Target Milestone: --- → Firefox 16
(Assignee)

Comment 7

5 years ago
Created attachment 636888 [details] [diff] [review]
Patch v3
Attachment #633793 - Attachment is obsolete: true
Attachment #636888 - Flags: review?(felipc)
Attachment #636888 - Flags: review?(felipc) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea8be9af9183
(Reporter)

Updated

5 years ago
Whiteboard: [blocking-webrtdesktop1+] → [blocking-webrtdesktop1+], [qa+]

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/ea8be9af9183
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Comment 10

5 years ago
Looks good on Windows.
(Reporter)

Comment 11

5 years ago
Looks good on Linux. Verified.
Status: RESOLVED → VERIFIED
Whiteboard: [blocking-webrtdesktop1+], [qa+] → [blocking-webrtdesktop1+], [qa!]
(Reporter)

Updated

5 years ago
QA Contact: jsmith
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.