Add contact support to form autocomplete (Maemo)

VERIFIED FIXED

Status

Firefox for Android Graveyard
General
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: mfinkle, Assigned: mfinkle)

Tracking

Bug Flags:
in-testsuite +

Details

(URL)

Attachments

(1 attachment, 2 obsolete attachments)

We can read the device address book and use the data to match a person's name to a email addresses. Those addresses can be shown along with form autocomplete data.

This bug will add support for Maemo, using libebook, which also works on desktop Linux

See: https://wiki.mozilla.org/Mobile/Projects/Contacts
(Assignee)

Updated

8 years ago
Assignee: nobody → mark.finkle
Created attachment 457955 [details] [diff] [review]
wip 1

This patch adds much of the infrastructure to support "contacts in form autocomplete":
* Adds js-ctype access library to the libebook address book
* Adds an override to the Form Autocomplete component which uses the contact lookup
* Adds build stuff to get modules into build

I still need to refactor the code so it works well on other platforms, not just Linux and Maemo. I'm also interested to learn how we can get support for Android and Windows (if we care). Mac support seems easy to add.
Even though this patch depends on bug 552828, you should still be able to see the contacts dumped to the console when fennec starts up - just some debugging code in the WIP.
Depends on: 552828
(In reply to comment #2)
> Even though this patch depends on bug 552828, you should still be able to see
> the contacts dumped to the console when fennec starts up - just some debugging
> code in the WIP.

And browser.tabs.remote = false, works well too
Created attachment 459311 [details] [diff] [review]
wip 2

* Fixes some bugs
* Separates the linux contacts code into a "provider"
* Merges the default autocomplete into the contact autocomplete per design. The last patch only used one or the other.

Next patch will add some tests - using a mock provider
Attachment #457955 - Attachment is obsolete: true
Created attachment 461480 [details] [diff] [review]
patch

This patch supports the design and has tests, which pass as long as bug 579178 is fixed. Without the fix, our FormAutoComplete component is not registered.

Contacts work on Ubuntu and N900. We can add other platforms in other bugs. I need to check to see what libebook version is shipped on the N810, in case we want to support it too.
Attachment #459311 - Attachment is obsolete: true
Attachment #461480 - Flags: review?(21)
Comment on attachment 461480 [details] [diff] [review]
patch

>diff --git a/components/FormAutoComplete.js b/components/FormAutoComplete.js
>new file mode 100644
>--- /dev/null
>+++ b/components/FormAutoComplete.js
>+
>+function _() {
>+  return; // comment out for verbose debugging
>+  let msg = Array.join(arguments, " ");
>+  dump(msg + "\n");
>+  Cu.reportError(msg);
>+}

nit: please use a real name for the function

>+
>+// Lazily get the base Form AutoComplete Search
>+XPCOMUtils.defineLazyGetter(this, "FAC", function() {
>+  return Components.classesByID["{c11c21b2-71c9-4f87-a0f8-5e13f50495fd}"]
>+                   .getService(Ci.nsIFormAutoComplete);
>+});
>+
>+XPCOMUtils.defineLazyGetter(this, "Contacts", function() {
>+  Cu.import("resource:///modules/contacts.jsm");
>+  return Contacts;
>+});
>+
>+function FormAutoComplete() {
>+  _("new FAC");
>+}
>+
>+FormAutoComplete.prototype = {
>+  classDescription: "Form AutoComplete Plus",
>+  classID: Components.ID("{cccd414c-3ec2-4cc5-9dc4-36c87cc3c4fe}"),
>+  QueryInterface: XPCOMUtils.generateQI([Ci.nsIFormAutoComplete]),
>+
>+  // Specify the html5 types that we want and some values to guess
>+  contactTypes: {
>+    email: /^(?:.*(?:e-?mail|recipients?).*|(send_)?to(_b?cc)?)$/i,
>+    tel: /^(?:tel(?:ephone)?|.*phone.*)$/i
>+  },

Mounir, do you know if any other html5 types can be of interest here?

>+
>+  checkQueryType: function checkQueryType(name, field) {

Nit: checkQueryType(aName, aField)

>+    // If we have an input field with the desired html5 type, take it!
>+    if (field != null) {
>+      let type = field.getAttribute("type");
>+      if (this.contactTypes[type] != null)
>+        return type;
>+    }

>+  findContact: function findContact(query, type, result, dupCheck) {

Nit: findContact(aQuery, aType, aResult, aDuplicateCheck)

>+
>+        for each (let suggestion in suggestions) {
>+          if (dupCheck[suggestion]) continue;

Nit: if (aDuplicateCheck[suggestion])
       continue;

>+
>+  autoCompleteSearch: function autoCompleteSearch(name, query, field, prev) {

nit: autocompleteSearch(aName, aQuery, aField, aPrev)

>diff --git a/components/MobileComponents.manifest b/components/MobileComponents.manifest
>--- a/components/MobileComponents.manifest
>+++ b/components/MobileComponents.manifest
>@@ -77,8 +77,12 @@ contract @mozilla.org/autocomplete/searc
> component {f570982e-4f15-48ab-b6a0-ed851ac551b2} AutoCompleteCache.js
> contract @mozilla.org/browser/autocomplete-observer;1 {f570982e-4f15-48ab-b6a0-ed851ac551b2}
> category bookmark-observers BrowserBookmarkObserver @mozilla.org/browser/autocomplete-observer;1
> 
> # AddonUpdateService.js
> component {93c8824c-9b87-45ae-bc90-5b82a1e4d877} AddonUpdateService.js
> contract @mozilla.org/browser/addon-update-service;1 {93c8824c-9b87-45ae-bc90-5b82a1e4d877}
> category update-timer AddonUpdateService @mozilla.org/browser/addon-update-service;1,getService,auto-addon-background-update-timer,extensions.autoupdate.interval,86400
>+
>+# FormAutoComplete.js
>+component {cccd414c-3ec2-4cc5-9dc4-36c87cc3c4fe} FormAutoComplete.js
>+contract @mozilla.org/satchel/form-autocomplete;1 {cccd414c-3ec2-4cc5-9dc4-36c87cc3c4fe}

With patches from bug 579178 this manifest works for me.

>+++ b/modules/contacts.jsm
>+
>+Cu.import("resource://gre/modules/ctypes.jsm");
>+
>+let Contacts = {
>+  _providers: [],
>+  _contacts: [],
>+
>+  _load: function _load() {
>+    this._contacts = [];
>+
>+    this._providers.forEach(function(provider) {
>+      this._contacts = this._contacts.concat(provider.getContacts());
>+    }, this)
>+  },
>+
>+  init: function init() {
>+    this._load();
>+  },
>+
>+  addProvider: function(aProvider) {
>+    this._providers.push(aProvider);
>+    this._load();
>+  },
>+
>+  find: function find(aMatch) {
>+    let results = [];
>+
>+    if (!this._contacts.length)
>+      return results;
>+
>+    for (let field in aMatch) {
>+      // Simple string-only partial matching
>+      let match = aMatch[field];
>+      this._contacts.forEach(function(aContact) {
>+        if (field in aContact && aContact[field].indexOf(match) != -1)
>+          results.push(aContact);
>+      });
>+    }
>+    return results;
>+  }
>+};

nit: Since the list is quite static I wouldn't mind seeing a "refresh" function doing the same as "init" for now

>+
>+#ifdef XP_UNIX
>+Cu.import("resource:///modules/linuxTypes.jsm");
>+
>+function EBookProvider() {
>+    EBook.init();
>+}

nit: indentation is wrong


>diff --git a/modules/linuxTypes.jsm b/modules/linuxTypes.jsm
>new file mode 100644
>--- /dev/null
>+++ b/modules/linuxTypes.jsm
>+  init: function ebook_init() {
>+    if (this.lib)
>+      return;
>+
>+    GLib.init();
>+
>+    try {
>+      // Shipping on N900
>+      this.lib = ctypes.open("libebook-1.2.so.5");
>+    } catch (e) {
>+      try {
>+        // Shipping on Ubuntu
>+        this.lib = ctypes.open("libebook-1.2.so.9");

I wonder if we should not use a pref for the name of the lib, it will allow us to avoid the try nesting and also allow people using a different lib to use it if their libs is compatible.

browser_contacts.js tests are green on my linux box.
Attachment #461480 - Flags: review?(21) → review+
Mounir, can you check if we need to handle more HTML5 types than email/tel for contacts?
Filed bug 583238 to be able to safely pass more context to the FAC search
pushed:
http://hg.mozilla.org/mobile-browser/rev/14c35dd49a71
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(In reply to comment #7)
> Mounir, can you check if we need to handle more HTML5 types than email/tel for
> contacts?

No. Except if you want to fill <input type='url'> with your friends websites which doesn't really sound useful ;)

>+    try {
>+      // Shipping on N900
>+      this.lib = ctypes.open("libebook-1.2.so.5");
>+    } catch (e) {
>+      try {
>+        // Shipping on Ubuntu
>+        this.lib = ctypes.open("libebook-1.2.so.9");

Can't you open "libebook.so" instead? libebook.so should point to the correct version.

I quickly look at the patch and it looks like you get the type with getAttribute and then compares it to 'email' and 'tel' but the type attribute isn't case sensitive so type='EMAIL' should work too. You can use .type which returns the type in lower case but you will have to depend on bugs implementing that (should land for ff4).
(Assignee)

Updated

8 years ago
Flags: in-testsuite+
Verified contacts support landed on n900.  Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:2.0b4pre) Gecko/20100810 Namoroka/4.04pre Fennec/2.0a1pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.