Closed Bug 968738 Opened 7 years ago Closed 7 years ago
Bookmarks opened from home screen are not marked as bookmarked
Environment: Device: Google Nexus 7 (Android 4.4.2); Build: Nightly 30.0a1 (2014-02-04); Steps to reproduce: 1. Load any page (e.g. ebay.com); 2. Bookmark the page; 3. On the toaster notification tap on Options; 4. Tap on Add to Home Screen; 5. Go to the home screen and tap on the bookmark; 6. Check if the opened page is bookmarked. Expected result: The page appears as bookmarked. Actual result: The page does not appear as bookmarked.
I'm assuming this short-circuiting logic was to added prevent flickering favicons/page titles, but we don't want to miss other state updates.
An overview of what's happening here: 1) All tabs loaded from Java are created as stubs, meaning their URLs are set before Gecko responds with a Tab:Added message (http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/Tabs.java#758). 2) We get a location changed event, but we return early because the URL is the same (http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/Tab.java#612). 3) Consequently, updateBookmark never gets called.
Minor update that adds an updateBookmark() call to the Tab constructor so that we don't need to rely on the location change event for updates. This means we can avoid doing extra updateBookmark() calls if oldUrl == url, like we had before.
Comment on attachment 8373545 [details] [diff] [review] Don't short-circuit most tab updates if the URL is the same, v2 Review of attachment 8373545 [details] [diff] [review]: ----------------------------------------------------------------- Imma say this looks good to me, under the assumption that you've tested the crap out of this and we have several weeks to notice if titles or icons don't get updated correctly. Thanks for doing the digging! ::: mobile/android/base/Tab.java @@ +419,4 @@ > } > > + mBookmark = BrowserDB.isBookmark(getContentResolver(), url); > + mReadingListItem = BrowserDB.isReadingListItem(getContentResolver(), url); /me files bug to only hit the DB *once* on every tab load. /me files bug to not hit the DB on every tab load.
Attachment #8373545 - Flags: review?(rnewman) → review+
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
You need to log in before you can comment on or make changes to this bug.