1312375 - Should be able to switch persistent-storage permission status for site

Status: RESOLVED WONTFIX

(Firefox :: Settings UI, defect)

Description

[Fischer [:Fischer]]

Should be able to to switch persistent-storage permission status between persistent and non-persistent for site in Settings of Site Data

Comment 1

[Tom Tung [:tt, :ttung]]

Hi Mark,

After discussion with Jan and Shawn, we should consider several situations before starting to implement this. We would like to look for the feedback from UX guys. I list them below inline. 

- How do we deal with the UI for the latency among transferring the persistent storage into best-effort? This concern raises because |un-persist| operation is asynchronous and we should consider the latency for the UI display. Currently, UI seems only to take care the result from  PermissionManager and doesn't wait for the result from QuotaManager. It may also have an another problem for synchronizing them.

- What should we do if a |un-persist| operation fails?  Shall we roll back the status and show the error message to the user, or delete the un-persisting storage?

- What should we do if the |un-persist| operation fails while transferring persisted multiple storages (origins) to best-effort? (Should we roll back all of them, or continue to |un-persist| the rest of them, or ... etc ?)

Comment 2

[Mark Liang(:mark_liang)]

(In reply to Tom Tung [:tt] from comment #1)
> Hi Mark,
> 
> After discussion with Jan and Shawn, we should consider several situations
> before starting to implement this. We would like to look for the feedback
> from UX guys. I list them below inline. 
> 
> - How do we deal with the UI for the latency among transferring the
> persistent storage into best-effort? This concern raises because
> |un-persist| operation is asynchronous and we should consider the latency
> for the UI display. Currently, UI seems only to take care the result from 
> PermissionManager and doesn't wait for the result from QuotaManager. It may
> also have an another problem for synchronizing them.
> 
> - What should we do if a |un-persist| operation fails?  Shall we roll back
> the status and show the error message to the user, or delete the
> un-persisting storage?
> 
> - What should we do if the |un-persist| operation fails while transferring
> persisted multiple storages (origins) to best-effort? (Should we roll back
> all of them, or continue to |un-persist| the rest of them, or ... etc ?)

I think it would be ideal if we can avoid all the operation fails without rolling back the status.

I'd propose that once a user saved all the changes using the dropdown UI (changing to persistent or temporary), the UI should change immediately without waiting for the Quota Manager, and if the operation fails we should just delete the un-persisting storage. The same applies to multiple storages operations: if one of the operation fails we just delete the un-persisting storage and make sure the whole operation works.

Any drawbacks or feasibility problems for this implementation?

Comment 3

[Tom Tung [:tt, :ttung]]

(In reply to Mark Liang(:mark_liang) from comment #2)
> Any drawbacks or feasibility problems for this implementation?

I cannot find any feasibility problem right now, and it looks good to me.

Comment 4

[Jan Varga [:janv]]

Not sure if this design is the best, we should have a clear idea how the new storage prefs should work. For example, do we know what origins we will offer to the user ? Some origins can be in use, etc.
Someone should take all the UX diagrams/screenshots and analyze it. The output of this analysis should be a workflow and what quota manager APIs need to be changed or added (we might even find out that we need to change UX a bit). Then we can start implementing stuff in quota manager, etc.

Comment 5

[Anne (:annevk)]

Perhaps rather than exposing it as a setting to flip back, we should expose it as "Clear storage" in the UI? That more clearly indicates what will happen.

Comment 6

[Jan Varga [:janv]]

(In reply to Anne (:annevk) from comment #5)
> Perhaps rather than exposing it as a setting to flip back, we should expose
> it as "Clear storage" in the UI? That more clearly indicates what will
> happen.

Yeah, especially give comment 2, I wanted to propose "Unpersist or clear", but that can be confusing to the user. On the other hand, clicking on unpersist and then the origin is suddenly cleared may appear confusing too.
If we decide to go for "Clear storage" then the new unpersist method in quota manager can be optimized for that.
Anyway, I would still like to see a diagram of entire workflow or at least a description of it. It's still not clear to me how we will offer origins for unpersisting/clearing. There's no quota manager API for that right now.

Comment 7

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Hey Mark,
Can you provide UI flow charts that you currently propose? So we can discuss further based on that page. Otherwise, we might be blocked here since Tom is working on Bug 1301276 (quota manager unpersist api) and we don't have clear pictures to have complete API design. I believe the debate on "unpersist" and "clear storage" happened for a while ago. We can discuss further on next Wednesday on Vidyo.

Comment 8

[Mark Liang(:mark_liang)]

(In reply to Shawn Huang [:shawnjohnjr] from comment #7)
> Hey Mark,
> Can you provide UI flow charts that you currently propose? So we can discuss
> further based on that page. Otherwise, we might be blocked here since Tom is
> working on Bug 1301276 (quota manager unpersist api) and we don't have clear
> pictures to have complete API design. I believe the debate on "unpersist"
> and "clear storage" happened for a while ago. We can discuss further on next
> Wednesday on Vidyo.

For the current proposal, the description is here: https://mozilla.invisionapp.com/share/XUALQSRBK

It says "delete the origin's old data after the user closes the tab" 
and also "if Firefox failed to change an origin from persistent to non-persistent, show the status that aligns with user's choice and fixes it in the background." 

I think Tina wrote those down as a note in our early discussion on changing storage status. I'll have more discussion with her to see if we can illustrate the entire workflow for unpersisting/clearing situations.

Thanks for the feedback.

Comment 9

[Fischer [:Fischer]]

As the discussion in the bug 1301276[1], this feature will not be implemented so mark it as WONTFIX.
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1301276#c37