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

RESOLVED FIXED in Firefox 42

Status

()

RESOLVED FIXED
3 years ago
3 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)

(Assignee)

Description

3 years ago
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.
(Assignee)

Updated

3 years ago
See Also: → bug 1183559
(Assignee)

Comment 1

3 years ago
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)

Updated

3 years ago
Assignee: nobody → s.kaspari
Status: NEW → ASSIGNED
(Assignee)

Comment 2

3 years ago
Created attachment 8642435 [details] [diff] [review]
1183069-legacy-browser.patch

This patch copies the removed fields from android.provider.Browser to a new class called LegacyBrowserProvider.
(Assignee)

Comment 3

3 years ago
Created attachment 8642443 [details] [diff] [review]
1183069-legacy-browser-v2.patch
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+
(Assignee)

Comment 6

3 years ago
(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
(Assignee)

Comment 7

3 years ago
Created attachment 8642926 [details] [diff] [review]
1183069-legacy-browser-v3.patch

(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)
(Assignee)

Updated

3 years ago
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+
(Assignee)

Comment 11

3 years ago
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
Last Resolved: 3 years ago
status-firefox42: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
You need to log in before you can comment on or make changes to this bug.