Android M: Multiple fields and methods in android.provider.Browser removed

RESOLVED FIXED in Firefox 42

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: sebastian, Assigned: sebastian)

Tracking

(Blocks 1 bug)

42 Branch
Firefox 42
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

A lot of fields and methods have been removed from android.provider.Browser. Our import code (AndroidImport) and LocalBrowserDB seem to depend on Browser.BOOKMARKS_URI and Browser.BookmarkColumns. These fields are now all gone with Android M Preview 2. Probably the whole content provider does not exist anymore.
See Also: → 1183559
We either will have to copy these fields to our code base to build with the Android M SDK and continue to support this feature ("Import from Android") on older versions (see bug 1183559) or remove the feature completely.
Assignee: nobody → s.kaspari
Status: NEW → ASSIGNED
Posted patch 1183069-legacy-browser.patch (obsolete) — Splinter Review
This patch copies the removed fields from android.provider.Browser to a new class called LegacyBrowserProvider.
Attachment #8642435 - Attachment is obsolete: true
Attachment #8642443 - Flags: review?(rnewman)
I guess the API docs haven't been updated to show deprecations in 22 yet, mm?
Comment on attachment 8642443 [details] [diff] [review]
1183069-legacy-browser-v2.patch

Review of attachment 8642443 [details] [diff] [review]:
-----------------------------------------------------------------

Well done for finding two bugs :)

::: mobile/android/base/db/LocalBrowserDB.java
@@ +41,5 @@
>  import org.mozilla.gecko.gfx.BitmapUtils;
>  import org.mozilla.gecko.mozglue.RobocopTarget;
>  import org.mozilla.gecko.sync.Utils;
>  import org.mozilla.gecko.util.GeckoJarReader;
> +import org.mozilla.gecko.util.LegacyBrowserProvider;

... and this becomes unnecessary.

@@ +889,5 @@
>      private void addBookmarkItem(ContentResolver cr, String title, String uri, long folderId) {
>          final long now = System.currentTimeMillis();
>          ContentValues values = new ContentValues();
>          if (title != null) {
> +            values.put(LegacyBrowserProvider.BookmarkColumns.TITLE, title);

This is wrong. We should be using Bookmarks.TITLE here!

@@ +992,5 @@
>      @Override
>      @RobocopTarget
>      public void updateBookmark(ContentResolver cr, int id, String uri, String title, String keyword) {
>          ContentValues values = new ContentValues();
> +        values.put(LegacyBrowserProvider.BookmarkColumns.TITLE, title);

Bookmarks.TITLE.

::: mobile/android/base/util/LegacyBrowserProvider.java
@@ +1,4 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +package org.mozilla.gecko.util;

Newline before the package, but can we just put this as a private static inner class in AndroidImport.java? It shouldn't be used anywhere else in the codebase.
Attachment #8642443 - Flags: review?(rnewman) → feedback+
(In reply to Richard Newman [:rnewman] from comment #4)
> I guess the API docs haven't been updated to show deprecations in 22 yet, mm?

The web version hasn't been updated yet, but you can download the API docs for the preview version: http://developer.android.com/preview/download.html#docs
(In reply to Richard Newman [:rnewman] from comment #5)
> Well done for finding two bugs :)

Nice. I pay too much attention to the code. ;)

> Newline before the package, but can we just put this as a private static
> inner class in AndroidImport.java? It shouldn't be used anywhere else in the
> codebase.

Yeah, I updated the patch and moved the class inside AndroidImport.
Attachment #8642926 - Flags: review?(rnewman)
Attachment #8642443 - Attachment is obsolete: true
Comment on attachment 8642926 [details] [diff] [review]
1183069-legacy-browser-v3.patch

Review of attachment 8642926 [details] [diff] [review]:
-----------------------------------------------------------------

I kinda want to conditionalize the whole importer on <M, so that if we do an M split we kill all of that code for free. But this patch is good :)
Attachment #8642926 - Flags: review?(rnewman) → review+
url:        https://hg.mozilla.org/integration/fx-team/rev/2e605777f948abaa33d6c02b68009f360ccd21f2
changeset:  2e605777f948abaa33d6c02b68009f360ccd21f2
user:       Sebastian Kaspari <s.kaspari@gmail.com>
date:       Mon Aug 03 17:04:27 2015 +0200
description:
Bug 1183069 - Copy removed fields from android.provider.Browser to LegacyBrowserProvider. r=rnewman
https://hg.mozilla.org/mozilla-central/rev/2e605777f948
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
You need to log in before you can comment on or make changes to this bug.