939351 - (fx-autofill-backend) [meta] Form Autofill form backend

Status: NEW

(Toolkit :: Form Autofill, defect)

Description

[Andy McKay]

requestAutoComplete is an improvement to auto complete of form data.

http://www.chromium.org/developers/using-requestautocomplete

I haven't been able to find a W3C spec for it, but:

http://www.whatwg.org/specs/web-apps/current-work/multipage/association-of-controls-and-forms.html#attr-fe-autocomplete

Discusses autocomplete on an input field and how that is associated to the field.

This would help make payments easier on the web by allowing users to clearly store and access form fields.

Comment 1

[Richard Newman [:rnewman]]

http://www.mjt.me.uk/posts/falsehoods-programmers-believe-about-addresses/ writ large, in Google-colored neon letters against the night sky.

Comment 2

[Andy McKay]

I don't think there's anything in the requestAutoComplete that parses or makes assumptions about addresses. It just gives fields explicit names so they can be looked up across different sites.

Comment 3

[Richard Newman [:rnewman]]

(In reply to Andy McKay [:andym] from comment #2)
> I don't think there's anything in the requestAutoComplete that parses or
> makes assumptions about addresses. It just gives fields explicit names so
> they can be looked up across different sites.

It's the default set of explicit names that's the problem.

From the Chromium docs:

"there is a strict whitelist of fields that requestAutocomplete will populate".

"Shipping:        street-address, address-line1, address-line2, locality, region, country, postal-code"

What exactly about assuming two-line addresses, with a single locality, region, and postal code, isn't making an assumption about addresses? (Even "street-address" is defined as two comma-separated address lines.)

You might argue that the actual fields aren't anything to do with requestAutocomplete, but that's like talking about HTML without defining the default set of tags.

You might further argue that this is what websites already do, but that's advocating the codification and encouragement of exclusive and poorly thought-out restrictions. If you have an address that doesn't fit in the standard template, you not only aren't well-served by many existing websites, but furthermore we'll reinforce that by giving you a worse autocompletion behavior.

Alas, the shitty path has already been blazed.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

(Note, Chromium's doc is nowhere near a spec quality thing. We really need this stuff spec'ed properly somewhere, probably WhatWG/HTML)

Comment 5

[Evan Stade]

On the Chrome side, we're hard at work on internationalizing our dialog, such that the set of fields it can fill in is expanded. For example, we're adding support to fill in arbitrarily many address lines via street-address, and other fields like dependent locality (CEDEX), etc. are on the way. We more or less* plan to support the complete set of address-related fields in the autocomplete spec. Over the past Summer we attempted to improve i18n support in that spec with Ian Hixie's help. If there are further improvements necessary, please suggest them!

As far as the requestAutocomplete spec, that's here: <http://wiki.whatwg.org/wiki/RequestAutocomplete>. It really is quite short, and that's by design: requestAutocomplete is meant to be generally useful for any autocomplete type. It's just that Chrome's current use case is payments, and for now, that's all we have concrete plans to support. But you could imagine it being useful for other common web forms, and we wouldn't want to head that off at the spec level.

*what do I mean by more or less? Well, for example, it's hard to support first-name and last-name and also full-name, because both tokenizing names and concatenating names is hard, so unless we ask the users for their names twice, we don't see an easy way forward. So we've settled on full-name only for now. Generally though, we want to support as much of the address portion of the autocomplete spec as we can, and which makes sense in a payment-on-the-web context.

Comment 6

[Brian Nicholson (:bnicholson)]

Attachment: Implement toolkit requestAutocomplete components

I've had the past few weeks to work on the this and bug 946022, so it's probably a good time to get another feedback iteration on my progress so far.

This is still obviously a WIP, and I've sprinkled several comments throughout indicating things that need to be done. The biggest among those are storage and validation.

To track of things better, I created an etherpad at https://etherpad.mozilla.org/requestautocomplete to keep track of things, which I'll start filling in tomorrow. This will be a compilation of existing comments in this patch plus everything at http://www.chromium.org/developers/using-requestautocomplete that is currently unimplemented.

Comment 7

[Hixie (not reading bugmail)]

I didn't realise this was something Mozilla was waiting on a spec for, my bad. I'll get one out ASAP. The spec work will happen in https://www.w3.org/Bugs/Public/show_bug.cgi?id=22701 .

Comment 8

[Anne (:annevk)]

Brian, could you please follow these guidelines: https://wiki.mozilla.org/WebAPI/ExposureGuidelines

Comment 9

[Brian Nicholson (:bnicholson)]

Thanks for the heads up. For reference, here's the dev-platform post: https://groups.google.com/forum/#!topic/mozilla.dev.platform/F2mMPBme40I

Comment 10

[Hixie (not reading bugmail)]

http://www.whatwg.org/specs/web-apps/current-work/#dom-form-requestautocomplete

Comment 11

[Brian Nicholson (:bnicholson)]

Thanks, Hixie!

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8401711 [details] [diff] [review]
Implement toolkit requestAutocomplete components

Sorry about delay. Please ping me on IRC if I don't process
feedback queue fast enough. (it is always after reviews)

>+NS_IMETHODIMP
>+HTMLFormElement::RequestAutocomplete()
This doesn't look like a method implementing webidl method.
Drop NS_IMETHODIMP and just use void

>+++ b/content/html/content/src/HTMLFormElement.h
>@@ -403,16 +403,18 @@ public:
> #ifdef DEBUG
>   static void
>   AssertDocumentOrder(const nsTArray<nsGenericHTMLFormElement*>& aControls,
>                       nsIContent* aForm);
> #endif
> 
>   js::ExpandoAndGeneration mExpandoAndGeneration;
> 
>+  nsresult RequestAutocomplete();
void


>+[Constructor(DOMString type, optional AutocompleteErrorEventInit eventInitDict)]
>+interface AutocompleteErrorEvent : Event
>+{
>+  readonly attribute DOMString reason;
>+};
>+
>+dictionary AutocompleteErrorEventInit : EventInit
>+{
>+  DOMString reason = "";
>+};

Should not use DOMString but enum AutocompleteErrorReason.
I think we want AutocompleteErrorEvent enabled only if "dom.requestAutocomplete.enabled" is true
So, add Pref to the interface



>diff --git a/toolkit/components/satchel/AutofillContract.jsm b/toolkit/components/satchel/AutofillContract.jsm
>+function AutofillController() {}
>+
>+AutofillController.prototype = {
>+  _form: null,
>+  _win: null,
These properties are a bit worrisome. Are we making sure they are cleared on time, so that
AutofillController doesn't end up keeping objects alive too long?
(I *think* bug hueyfix would actually kill unnecessary js edges here, but better be safe than sorry. In C++ we've
have issues like bug 1005392)


>+  _cancel: function () {
>+    let evt = new this._win.AutocompleteErrorEvent("autocompleteerror", { bubbles: true, reason: "cancel" });



>+    this._form.dispatchEvent(evt);
>+    this._finish();
>+  },
>+
>+  requestAutocomplete: function (aForm) {
>+    let win = aForm.ownerDocument.defaultView;
>+
>+    if (!win) {
>+      Cu.reportError("requestAutocomplete called on element with no window");
>+      return;
>+    }
Hmm, do we really want to support
document.createElement("form").requestAutocomplete()?
I think C++ side should add some checks. Perhaps that form is in document, and that document has window.
(Better to check what the spec says here. It does say "form is not in a Document" and "form's Document is not fully active")


>+
>+    if (this._isActive || aForm.autocomplete === "off") {
>+      let evt = new win.AutocompleteErrorEvent("autocompleteerror", { bubbles: true, reason: "disabled" });
>+      aForm.dispatchEvent(evt);
This should happen using a task, not synchronously.



Some toolkit peer should look at this too.

Comment 13

[Brian Nicholson (:bnicholson)]

Attachment: Implement toolkit requestAutocomplete components, v2 WIP

An updated version of the rAc base that more closely conforms to the HTML spec. Not quite ready for review, but should be close. To keep things simple, I've removed all references to the storage/validation components so those can be tackled separately in other bugs.

Comment 14

[:Paolo Amadini]

Following up to the meeting we held yesterday, I'm about to file the individual bugs into which we determined we could break down the back-end implementation.

They'll be dependencies of this bug, and also of bug 1018317 which is the Firefox Desktop tracking bug about the work needed to reach a minimal version that can work on a demo website (but is far from being complete).

I'll also try to guess some "point estimates" for them, mainly for tracking purposes in the Desktop backlog process, and add them to the Desktop backlog for tracking, even though they are platform bugs and will probably be worked on by different teams.

Please let me know if they make sense to you, note that we can also file other bugs while we work, and determine whether they block bug 1018317 or not.

Comment 15

[:Paolo Amadini]

Brian, in particular I think bug 1020440 is already actionable by separating a very small subset of this patch, even without the implementation and UI glue.

Also, probably that bug is the only one requiring a DOM peer review before we start. Note that we will definitely need a more detailed DOM-level review later in the process, before we release. I filed bug 1020443 for that.

Comment 16

[:Paolo Amadini]

I filed various bugs, but none for the actual core of the implementation yet. Feel free to add those if you already have an idea, we can also discuss the plan in our next meeting.

Comment 17

[:Paolo Amadini]

(In reply to :Paolo Amadini from comment #16)
> I filed various bugs, but none for the actual core of the implementation yet.

In other words (for those who know about the Desktop estimation system), we already have 33 "points" worth of back-end work on the table, and none for the actual core of the feature :-)

Comment 19

[Matthew N. [:MattN]]

This bug is becoming a meta bug for the backend work which was split up.