Closed
Bug 1073646
Opened 8 years ago
Closed 8 years ago
[Settings] "Clear bookmarks data" does not work properly
Categories
(Firefox OS Graveyard :: Gaia::Homescreen, defect)
Tracking
(blocking-b2g:2.1+, b2g-v2.0 unaffected, b2g-v2.1 verified, b2g-v2.2 verified)
Tracking | Status | |
---|---|---|
b2g-v2.0 | --- | unaffected |
b2g-v2.1 | --- | verified |
b2g-v2.2 | --- | verified |
People
(Reporter: ychung, Assigned: crdlc)
References
()
Details
(Keywords: regression, Whiteboard: [2.1-exploratory-2][systemsfe])
Attachments
(4 files)
Description: The private data is not cleared when the user selects "Clear bookmarks data". The email is still logged in after the user cleared the bookmarks data. Repro Steps: 1) Update a Flame device to BuildID: 20140926000202 2) Search an email service provider (e.g. Gmail, Outlook, etc.) on the rocket bar. 3) When the search result shows up, long press, and select "Add to Home Screen", and Select "Done". 4) Go to Home screen, and open the added website. 5) Log into an email account. 6) Close the browser window. 7) Go to Settings > Browsing Privacy. 8) Select "Clear bookmarks data" and "Clear". 9) Re-open the email page from the Home screen. Actual: Email account is still logged in. (Still logged in after tapping Refresh button.) Expected: Email account is logged out as the user cleared the private data. Environmental Variables: Device: Flame 2.1 BuildID: 20140926000202 Gaia: 6a6ed9433fce47e76c07fd35bc5952acb108f4c8 Gecko: 7b09a378588c Version: 34.0a2 (2.1) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Repro frequency: 100% See attached: logcat, video http://youtu.be/GFzhjCIN9jE
Reporter | ||
Comment 1•8 years ago
|
||
This issue also reproduces on Flame 2.2: Flame 2.2 KitKat Base (319mb) Environmental Variables: Device: Flame 2.2 Master BuildID: 20140926040203 Gaia: a06714c555ca7068545f10b4437a16c14cd8e7f5 Gecko: 9e3d649b80a2 Version: 35.0a1 (2.2 Master) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0 The email account is still logged in after the user cleared the bookmark data. ====================================== This issue does NOT reproduces on Flame 2.0: Flame 2.0 KitKat Base (319mb) Device: Flame 2.0 BuildID: 20140926063008 Gaia: c1aa7829548e65360472c31544dbe2839eaf5be1 Gecko: 5a9f1f402425 Version: 32.0 (2.0) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 "Clear bookmarks data" is under App permissions on v.2.0. After clearing bookmarks data, the email account added to home screen is logged out when re-opening the page.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(kevin.tucker)
Reporter | ||
Updated•8 years ago
|
Flags: needinfo?(kevin.tucker) → needinfo?(ktucker)
Reporter | ||
Comment 2•8 years ago
|
||
https://bugzilla.mozilla.org/show_bug.cgi?id=938177#c17 According to Comment 17 and Comment 19 on Bug 938177, "Clear bookmarks data" can be merged with "Clear cookies and stored data", since bookmark has taken out of the new browser.
Whiteboard: [2.1-exploratory-2] → [2.1-exploratory-2][systemsfe]
Comment 3•8 years ago
|
||
Francis, can you take a look at this? Is this the plan to still merge the two buttons?
Flags: needinfo?(ktucker) → needinfo?(fdjabri)
Updated•8 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Comment 4•8 years ago
|
||
[Blocking Requested - why for this release]: The button currently does not function as far as we can tell so nominating this 2.1?
blocking-b2g: --- → 2.1?
Comment 5•8 years ago
|
||
Cristian - this might be bookmarks related. Could you help us debug this?
Flags: needinfo?(crdlc)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
Flags: needinfo?(crdlc)
Assignee | ||
Updated•8 years ago
|
Component: Gaia::Settings → Gaia::Homescreen
Assignee | ||
Comment 6•8 years ago
|
||
Kevin, The bug was in home screen because it was not consuming the "cleared" event. The datastore was cleared properly and there was not any issue in settings app (for this reason I moved the component in the bug from settings to homescreen). Arthur, I added a new panel BrowsingPrivacyPanel in marionette tests in settings app just in case you want to take a look at this change Thanks guys
Attachment #8498718 -
Flags: review?(kgrandon)
Attachment #8498718 -
Flags: review?(arthur.chen)
Comment 8•8 years ago
|
||
Comment on attachment 8498718 [details]
Github pull request
Looks good. There is just one problem with leaving two dividers in the code, it will create some weird interaction with dragdrop if we don't clean that up. Left a comment on github. Thanks for the quick patch!
Attachment #8498718 -
Flags: review?(kgrandon) → review+
Comment 9•8 years ago
|
||
(In reply to KTucker [:KTucker] from comment #3) > Francis, can you take a look at this? Is this the plan to still merge the > two buttons? Yes, that's still the desired outcome, so this bug is valid. Thanks, Francis
Flags: needinfo?(fdjabri)
Comment 10•8 years ago
|
||
This doesn't sound right to me. I don't there's anything to merge this button with. The current behavior of this button is to delete all bookmarks. TBH - I'm not sure it's a button we actually want in the product? The wording is also confusing - is this a major bug that we have not identified until now? It seems like we should remove this button.
Flags: needinfo?(fdjabri)
Comment 11•8 years ago
|
||
Comment on attachment 8498718 [details]
Github pull request
Clearing review requests for now. After speaking with UX - we should remove this button. It's harmful to have a button here which will remove all of the bookmarks on the device.
Cristian - Can you submit another patch to simply remove the button and logic surrounding it? Thanks!
Attachment #8498718 -
Flags: review?(arthur.chen)
Attachment #8498718 -
Flags: review+
Flags: needinfo?(crdlc)
Comment 12•8 years ago
|
||
(In reply to Francis Djabri [:djabber] from comment #9) > (In reply to KTucker [:KTucker] from comment #3) > > Francis, can you take a look at this? Is this the plan to still merge the > > two buttons? > > Yes, that's still the desired outcome, so this bug is valid. Thanks, Francis Just to clarify further: The intent behind comment 17 in Bug 938177 was that the old "Clear bookmarks data" that used to be under the App permissions settings should be merged with the "Clear cookies and stored data" button. Tapping this button should clear private data from sites, whether they are bookmarks or not. It seems what has happened however is that the "Clear bookmarks data" button has remained and it in fact deletes the users' bookmarks, not just the private data from them. So the "Clear bookmarks data" should be removed altogether as it was never requested. Jenny, could you please remove this button from the Settings spec? Thanks
Updated•8 years ago
|
Flags: needinfo?(fdjabri) → needinfo?(jelee)
Comment 13•8 years ago
|
||
Hi Francis, Oops! Looks like I misunderstood what you meant before. See attached for updated spec (also added a flow to show a toast after the clearing is complete). Thanks!
Flags: needinfo?(jelee)
Assignee | ||
Comment 14•8 years ago
|
||
Comment on attachment 8498718 [details]
Github pull request
Removed button and logic surrounding it
Attachment #8498718 -
Flags: review?(kgrandon)
Attachment #8498718 -
Flags: review?(arthur.chen)
Flags: needinfo?(crdlc)
Comment 15•8 years ago
|
||
Comment on attachment 8498718 [details]
Github pull request
Thank you for handling this, and sorry for the confusion earlier. Just a note about uplift - I think we'll probably want to prepare a patch without the string deletions for 2.1, so we don't break string freeze here.
Attachment #8498718 -
Flags: review?(kgrandon) → review+
Assignee | ||
Comment 16•8 years ago
|
||
With Artuhur's review I gonna implement a proper pr like this one without string deletion (In reply to Kevin Grandon :kgrandon from comment #15) > Comment on attachment 8498718 [details] > Github pull request > > Thank you for handling this, and sorry for the confusion earlier. Just a > note about uplift - I think we'll probably want to prepare a patch without > the string deletions for 2.1, so we don't break string freeze here.
Comment 17•8 years ago
|
||
Comment on attachment 8498718 [details]
Github pull request
LGTM, thanks for the patch!
Attachment #8498718 -
Flags: review?(arthur.chen) → review+
Assignee | ||
Comment 18•8 years ago
|
||
Merged in master: https://github.com/mozilla-b2g/gaia/commit/71f19789154e6d1e0e094c3970fb5f71fb7a8e3c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 19•8 years ago
|
||
Comment 20•8 years ago
|
||
(In reply to Cristian Rodriguez (:crdlc) from comment #19) > Created attachment 8499456 [details] > Github pull request for v2.1 (same patch without locales removed) Please request Gaia v2.1 approval on this patch.
Flags: needinfo?(crdlc)
Target Milestone: --- → 2.1 S6 (10oct)
Assignee | ||
Comment 21•8 years ago
|
||
Comment on attachment 8498718 [details]
Github pull request
[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: button to delete bookmarks which does not work
[Testing completed]: unit test/manual
[Risk to taking this patch] (and alternatives if risky): close to null
[String changes made]: No
NOTE: there is an specific pr for 2.1 which does not touch any locale
Attachment #8498718 -
Flags: approval-gaia-v2.1?
Flags: needinfo?(crdlc)
Updated•8 years ago
|
Attachment #8498718 -
Flags: approval-gaia-v2.1?
Updated•8 years ago
|
Attachment #8499456 -
Flags: approval-gaia-v2.1+
Comment 22•8 years ago
|
||
v2.1: https://github.com/mozilla-b2g/gaia/commit/f90385b521f29f426dff677e745f15d0dae5537b
Updated•8 years ago
|
Flags: in-moztrap?(mozillamarcia.knous)
Comment 23•8 years ago
|
||
This bug does NOT occur on the Flame 2.1 (319mb) and the Flame 2.2 (319mb) Flame 2.2 Master KK (319mb) (Full Flash) Device: Flame 2.2 BuildID: 20141010040202 Gaia: 1036b544b7e102592bd9fab95cd9317329ac1293 Gecko: 50b689feab5f Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf Version: 35.0a1 (2.2) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0 Flame 2.1 KK (319mb) (Full Flash) Environmental Variables: Device: Flame 2.1 BuildID: 20141010000201 Gaia: bc8eb493311c58f1f311a56b8b645b52bfbd2f71 Gecko: 72c13d8631ff Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf Version: 34.0a2 (2.1) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Result: Clear bookmarks data button has been removed.
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Updated•8 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in
before you can comment on or make changes to this bug.
Description
•