Adds support for clear cache #1517
Conversation
There was a problem hiding this comment.
A few comments:
- This probably belongs in Privacy and Security instead of developer options.
- Either it should be renamed to
Clear Cookies and Site Dataor we should give the user option as to what data is getting cleared.
I know a user requested that we give them the option to just clear the cache and I think that is reasonable.
app/src/common/shared/org/mozilla/vrbrowser/browser/engine/SessionStack.java
Outdated
Show resolved
Hide resolved
keianhzo
force-pushed
the
developer_cache_clear
branch
from
53dc347 to
58f2033
Compare
keianhzo
force-pushed
the
developer_cache_clear
branch
from
58f2033 to
199d47e
Compare
|
@MortimerGoro @bluemarvin I've updated with the change request and also made some refactoring to remove the intermediate session stack that is not really necessary and that simplifies the sessionstack code a bit. |
|
@keianhzo we should probably wait to land this until the UI spec has been updated? @jvonitter |
|
@bluemarvin There was never UI spec for this one, was just adding a clear cache button. You mean to have a more complete spec make the "clear" more granular? |
Right, I was asking if we should wait for the spec with more details or land this and follow up once there is a spec. |
|
yes please wait for the spec. it's on Nadja's list and it'll be done soon |
keianhzo
force-pushed
the
developer_cache_clear
branch
from
199d47e to
bfb8a3e
Compare
There was a problem hiding this comment.
I think we should land this as is for 1.4 and then address further UX in 1.5 @jvonitter ?
keianhzo
force-pushed
the
developer_cache_clear
branch
from
bfb8a3e to
f49a0a1
Compare
keianhzo
force-pushed
the
developer_cache_clear
branch
from
f49a0a1 to
5b9e1a6
Compare
app/src/main/res/values/strings.xml
Outdated
| @@ -290,6 +290,17 @@ | |||
| <!-- The string labels an On/Off switch in the developer options dialog and is used to toggle enabling Servo. --> | |||
| <string name="developer_options_servo">Enable Servo</string> | |||
|
|
|||
| <!-- The string labels the description text for the clear cookies and site data button in the | |||
| privacy options dialog. --> | |||
| <string name="developer_options_clear_cache_cookies_site_description">Cookies & SiteData</string> | |||
There was a problem hiding this comment.
Should SiteData be Site Data?
|
We have to leave out the descriptions from the spec as GV doesn't currently expose any API for getting the cache used disk space. |
|
I'm the original requester for this feature and I'm a little concerned. Am I right that you will have one button that clears cache, history, and cookies, much as with Mobile Safari? If so, in my experience that would be a mistake. A major reason for clearing the cache is so you can reload a page and be certain you have the most current content. That is especially important when developing and is just about the first thing any support or FAQ tells a user to do when troubleshooting. When something doesn't work as expected you can be certain it isn't just because some old code is still being used. If you nuke everything just to clear the cache you force users to retype URLs, which is tedious and error-prone, especially in a VR interface, or to search for lost URLs, which can be difficult. I suggest you use the approach of all other browsers (except mobile Safari) and give users control over just what they want to clear. Look at mobile FireFox as a model. Thanks for your consideration. Really excited to see how FxR is developing! |
|
There are two buttons, one clears cache, one clears cookies and data. |
|
Excellent. Thanks for the clarification. |
Fixes #1454 Adds support for clearing the cache. At the moment just clears all the caches until with have a more complete spec to do it more granular. Also I've refactored sessions code and fixed: