Closed Bug 1025441 Opened 5 years ago Closed 5 years ago
[Homescreen] Removed bookmarks show up again when restarting the phone
Steps to reproduce: 1. Open browser 2. Go to a website and tap on the star icon on the menu bar 3. Tap on add to home screen 4. Confirm The icon shows up on the homescreen as expected 5. Long tap on the icon to go into edit mode in the home screen 6. Tap on the X to remove the recently added bookmark The icon disappears as expected. Now reboot the phone and you will see the icon back in the homescreen. Back from the death
The patch overrides _decorateIcon for bookmarks. It paints the icon circle as a background of the favicon that now seats in the middle. I feel I have not spend enough time polishing and testing the patch to mark if for review. I can jump on this again on Monday but feel free to take it from here if you want to get it done any earlier.
Comment on attachment 8440258 [details] [review] Pull request Sorry PR doesn't belong to this bug
Attachment #8440258 - Attachment is obsolete: true
I might look into this, but I think the ni? was for the other patch :) Thanks!
blocking-b2g: --- → 2.0?
QA Whiteboard: [VH-FL-blocking+][VH-FC-blocking+]
We've also had reports of people being unable to delete bookmarks. Still trying to find a solid STR here. I filed bug 1026265 today, not sure if it's related.
See Also: → 1026265
I gonna take a look at this today
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
Well I can reproduce the bug and I've detected that the bug is the vertical homescreen. When I did the Bookmark app I implemented a Bookmark Reader app  and if you open this app the bookmark is not there (it means that the bookmark datastore is ok). Maybe the bookmark persists in the indexedDB of vh.  https://github.com/mozilla-b2g/gaia/tree/master/dev_apps/bookmarks-reader
I removed the "see also" because it was not related to this one
Comment on attachment 8441207 [details] Github pull request I think this is probably fine to land, I don't think this should be needed though. Synchronize should only be needed if other apps modify bookmark content. Ideally the record should be removed and saved on click.
Attachment #8441207 - Flags: review?(kgrandon) → review+
This is landed, but I don't think we've fixed the root cause, though it may mask it: https://github.com/mozilla-b2g/gaia/commit/0b77e55360ee62f4fe593bcd09606c9e7bbb3e1b
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
I see that it works like this. When you click to remove the icon this is removed from grid and datastore as well. Maybe we have to study to update indexedDB too. Right? (In reply to Kevin Grandon :kgrandon from comment #9) > Comment on attachment 8441207 [details] > Github pull request > > I think this is probably fine to land, I don't think this should be needed > though. > > Synchronize should only be needed if other apps modify bookmark content. > Ideally the record should be removed and saved on click.
After the datastore removal, we should fall into this codepath: https://github.com/mozilla-b2g/gaia/blob/master/apps/verticalhome/js/sources/bookmark.js#L88 Which looks like it may not actually be hooked up to indexedDB anymore. Should've written tests for this stuff :)
If the root of the issue remains unfixed. Is there a bug to keep track of it?
I am looking into it, but I assume that the fix will be the same as what we do in bug 1026467, so let's investigate there. In any case, this should no longer be an issue.
Verified on master.
Status: RESOLVED → VERIFIED
This issue has been verified successfully on Flame2.0&2.1 Verify video:"verify_1025441.mp4". Flame2.0 build Gaia-Rev 824a61cccec4c69be9a86ad5cb629a1f61fa142f Gecko-Rev https://hg.mozilla.org/mozilla-central/rev/acde07cb4e4d Build-ID 20141125040209 Version 36.0a1 Device-Name flame FW-Release 4.4.2 FW-Incremental eng.cltbld.20141125.113029 FW-Date Tue Nov 25 11:30:43 EST 2014 Bootloader L1TC00011880 Flame2.1 build: Gaia-Rev db2e84860f5a7cc334464618c6ea9e92ff82e9dd Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/211eae88f119 Build-ID 20141126001202 Version 34.0 Device-Name flame FW-Release 4.4.2 FW-Incremental eng.cltbld.20141126.033519 FW-Date Wed Nov 26 03:35:30 EST 2014 Bootloader L1TC00011880
You need to log in before you can comment on or make changes to this bug.