1201642 - Make UI for clearing existing private data items individually

Status: RESOLVED FIXED

(Firefox for iOS :: General, defect)

Description

[Brian Nicholson (:bnicholson)]

Right now, we support clearing history, cookies, cache, logins, and site data. A good start to bug 1191450 would be to provide a UI for clearing these things individually.

Comment 1

[Brian Nicholson (:bnicholson)]

Attachment: Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1031

Comment 2

[Robin Andersen [:tecgirl]]

Comment on attachment 8656742 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1031

The UI should follow what we're doing in Settings, with top and bottom borders to help separation, and no subsequent dividers on the cells below the "Clear Private Data" button.

As far as the toast is concerned, something would be nice, even if for a couple of seconds before it bumps you back to Settings. I would not leave the user on this screen after they've completed the action.

Comment 3

[Brian Nicholson (:bnicholson)]

Hey Robin, updated the cell styles. For the post-click toast/alert, do you know what we want now, or should I file a separate follow-up for us to decide on that UX?

Comment 4

[Robin Andersen [:tecgirl]]

(In reply to Brian Nicholson (:bnicholson) from comment #3)
> Hey Robin, updated the cell styles. For the post-click toast/alert, do you
> know what we want now, or should I file a separate follow-up for us to
> decide on that UX?

Brian,

Go ahead and file separately, because this looks good, works, and is good for now. I do believe it can improved by either an alert with more information about what you're clearing or a self-destructing toast that takes you back to Settings (like you have now).

Comment 5

[Brian Nicholson (:bnicholson)]

(In reply to Robin Andersen [:tecgirl] from comment #4)
> Go ahead and file separately, because this looks good, works, and is good
> for now. I do believe it can improved by either an alert with more
> information about what you're clearing or a self-destructing toast that
> takes you back to Settings (like you have now).

Thanks! Filed bug 1202093.

Comment 6

[Brian Nicholson (:bnicholson)]

Comment on attachment 8656742 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1031

Plan to land this along with bug 1170690.

Comment 7

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8656742 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1031

Nice patch series.  The preamble guiding the reviewer is appreciated!

Comment 8

[Brian Nicholson (:bnicholson)]

Merged 7eb679a.

Comment 9

[Stefan Plewako [:stef]]

https://github.com/mozilla/firefox-ios/commit/e714458f0368969fabadc6874b595fb9a0500bad#diff-4b55209fd9d15aff4987f5bd3aa91e20R37
https://github.com/mozilla/firefox-ios/commit/e714458f0368969fabadc6874b595fb9a0500bad#diff-4b55209fd9d15aff4987f5bd3aa91e20R61

"Clear Private Data" string shouldn't be one string but at least two (if not three).

Comment 10

[Francesco Lodolo [:flod]]

Not sure why the needinfo for me, but I can confirm this is an issue, and I'd suggest to file a new bug for it: while English might be fine with "Clear Private Data" used as both title and button label, other languages are not. One string, two different contexts that I can see.

Sadly, as discussed in the past, there's no easy solution since we use the actual string as string identifier.

Comment 11

[Brian Nicholson (:bnicholson)]

Filed bug 1205803; hopefully we can address this by using a separate tableName.

Comment 12

[Stefan Plewako [:stef]]

(In reply to Brian Nicholson (:bnicholson) from comment #11)
> Filed bug 1205803; hopefully we can address this by using a separate
> tableName.

Unfortunately not,
https://github.com/splewako/firefox-ios-l10n-en/commit/abb161b83bc54452e3c4206b7c168b5125240578#diff-f0455d5a9fe1f9eef6bc4ab45eefa508R88

Comment 13

[Brian Nicholson (:bnicholson)]

(In reply to Stefan Plewako [:stef] from comment #12)
> Unfortunately not,
> https://github.com/splewako/firefox-ios-l10n-en/commit/
> abb161b83bc54452e3c4206b7c168b5125240578#diff-
> f0455d5a9fe1f9eef6bc4ab45eefa508R88

Hm, looks like it did actually separate two of them (see [1] and [2]) -- but I forgot that the same string appears on the same settings panel (as the nav title and a button). We could create another tableName specifically for one of those, though that feels like a hack :\

[1] https://github.com/splewako/firefox-ios-l10n-en/commit/abb161b83bc54452e3c4206b7c168b5125240578#diff-f0455d5a9fe1f9eef6bc4ab45eefa508R85
[2] https://github.com/splewako/firefox-ios-l10n-en/commit/abb161b83bc54452e3c4206b7c168b5125240578#diff-f0455d5a9fe1f9eef6bc4ab45eefa508R352

Comment 14

[Francesco Lodolo [:flod]]

Can we use a string id (i.e. "clear_data_button") instead of the actual string as identifier?

See "String Key Best Practices" section
https://www.objc.io/issues/9-strings/string-localization/

Comment 15

[Brian Nicholson (:bnicholson)]

This seems like a good idea, though I'm not sure I fully understand the details. If we use a key, where do we map it to the actual string? Do we need to start manually maintaining an English string file like we do on Android?

Comment 16

[Brian Nicholson (:bnicholson)]

I guess that's more of a question for Stefan, in case he's familiar with this approach. I can bring this up in our next iOS meeting.

Comment 17

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Can we go with the table name solution here?

It is correct that we should have probably used indirect keys instead of (ab)using the English base language string as the identifier.

But I think it will be worse to change that now for just one string. It will also break our l10n flow because we are not prepared to deal with that.

If we feel we need to use a different approach then we should turn that plan that for a next release and do it consistently through the whole product.

Comment 18

[Francesco Lodolo [:flod]]

(In reply to Stefan Arentz [:st3fan] from comment #17)
> If we feel we need to use a different approach then we should turn that plan
> that for a next release and do it consistently through the whole product.

I don't that's a good idea: changing all IDs mean invalidating all the existing translations. If we ever decide to use proper string IDs, that should involve a gradual approach, i.e. using it only for new strings.