Closed Bug 1270740 Opened 7 years ago Closed 6 years ago

Remove requestAutocomplete

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: smaug, Assigned: jonathanGB, Mentored)

References

Details

(Keywords: dev-doc-complete, Whiteboard: btpp-backlog)

Attachments

(2 files, 1 obsolete file)

Bug 1270740 - Remove requestAutocomplete from toolkit/.
59 bytes, text/x-review-board-request
MattN
: review+
Details
Bug 1270740 - remove requestAutocomplete DOM code.
59 bytes, text/x-review-board-request
smaug
: review+
Details 
It is not enabled by default on any platform, as far as I see.
And Blink is apparently considering removing it since no one uses it
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/O9_XnDQh3Yk

The implementation is part DOM part Toolkit: FormManager.
MattN, do you know why we haven't shipped the feature, and is there any reason why we shouldn't just remove the code for it?
Flags: needinfo?(MattN+bmo)
Whiteboard: btpp-backlog
(In reply to Olli Pettay [:smaug] (pto-ish May 4 - May 8) from comment #1)
> MattN, do you know why we haven't shipped the feature, and is there any
> reason why we shouldn't just remove the code for it?

I just commented on the spec issue: https://github.com/whatwg/html/issues/1207#issuecomment-217560182

We haven't work on it recently though finishing it has been talked about many times over the last six months. Since it's a big project it has been hard getting resources committed and our team is focusing on quality, not new features. Previous work on it stopped as a higher priority project came up and it didn't get prioritized again.

I still think it's a good idea as it's a nice example of progressive enhancement on existing forms and it supports more than just payment information.

It's not clear to me why no one is using it. Is it because it was only a partial implementation by Chrome (payment-only) or that is was only one browser? Would us and/or another vendor shipping it change the adoption? I'm worried that the proposed alternative of the payment request API will lead to a worse/bad user and developer UX as it's not uncommon for sites to combine a checkout, registration and account creation flow in one form and the payment request API doesn't support non-payment information or username/passwords.
Flags: needinfo?(MattN+bmo)
Since it was removed from the spec I think it's fine to remove the form.requestAutocomplete() method but the parsing of the autocomplete attribute should remain for use by our password and form managers.

The toolkit UI and tests can be renamed to implement the payment request API next quarter.
I'm working on Form Autofill so will have a good sense of what can be removed vs. what will be reused soon.
(In reply to Matthew N. [:MattN] (behind on bugmail; PM if requests are blocking you) from comment #3)
> The toolkit UI and tests can be renamed to implement the payment request API
> next quarter.

This UI for this isn't going to be implemented until Q3 so we can remove it in the meantime and stuff can be relanded in its new form.

Remove all references to requestAutocomplete in https://dxr.mozilla.org/mozilla-central/search?q=requestautocomplete and https://dxr.mozilla.org/mozilla-central/search?q=path%3Arequestautocomplete&redirect=false except for the matches in nsFormAutoComplete.js which are unrelated.
* The whole toolkit/components/formautofill directory can be deleted for now.
* Use a separate commit for dom/ and toolkit/ changes since they require different code reviewers

Also remove the AutocompleteErrorEvent code which is a subset of https://dxr.mozilla.org/mozilla-central/search?q=AutocompleteError+-path%3Aobj-x&redirect=false

Care should be taken with the testing/web-platform/ files which are cross-browser tests. We should make sure we don't introduce failures but ideally the web platform tests (wpt) would have their requestAutocomplete and AutocompleteErrorEvent references removed since they should be removed from the specs.

Try syntax suggestion:
> try: -b do -p linux64 -u xpcshell,mochitests,web-platform-tests -t none
Assignee: nobody → jguillotteblouin
Mentor: MattN+bmo
Status: NEW → ASSIGNED
OS: Unspecified → All
Hardware: Unspecified → All
Summary: Consider removing requestAutocomplete → Remove requestAutocomplete
Version: 36 Branch → Trunk
Comment on attachment 8866979 [details]
Bug 1270740 - Remove requestAutocomplete from toolkit/.

https://reviewboard.mozilla.org/r/138576/#review141910

::: commit-message-3b96f:1
(Diff revision 2)
> +Bug 1270740 - Remove requestAutocomplete from toolkit/. r=mattn

Can you also remove ", such as requestAutocomplete.dtd," from browser/base/content/test/static/browser_misused_characters_in_strings.js in this commit (even though it's in browser/ since it's referring to a file that was already removed from toolkit/)
Attachment #8866979 - Flags: review?(MattN+bmo)
Comment on attachment 8866980 [details]
Bug 1270740 - remove requestAutocomplete DOM code.

https://reviewboard.mozilla.org/r/138578/#review141912

::: testing/web-platform/meta/html/dom/interfaces.html.ini:115
(Diff revision 3)
>    [Document interface: attribute onautocomplete]
>      expected: FAIL
>  
>    [Document interface: attribute onautocompleteerror]
>      expected: FAIL

After looking at the spec removal[1] I realized that I forgot about the autocomplete event which was also removed. Please remove these lines (assuming the tests were removed as they should have been).

[1] https://github.com/whatwg/html/commit/6a257aae619f85390eee20b47767f34887450fcd

::: testing/web-platform/meta/html/dom/interfaces.html.ini:163
(Diff revision 3)
>    [Document interface: iframe.contentDocument must inherit property "onautocomplete" with the proper type (94)]
>      expected: FAIL
>  
>    [Document interface: iframe.contentDocument must inherit property "onautocompleteerror" with the proper type (95)]
>      expected: FAIL

These too along with the many other references to "onautocomplete" and "onautocompleteerror" in this file.

(It seems like we may need to also fix testing/web-platform/tests/svg/interfaces.html since onautocomplete and onautocompleteerror probably shouldn't exist there anymore).

::: testing/web-platform/meta/html/dom/interfaces.html.ini:1084
(Diff revision 3)
>    [HTMLFormElement interface: operation requestAutocomplete()]
>      expected: FAIL
>  
>    [HTMLFormElement interface: document.createElement("form") must inherit property "requestAutocomplete" with the proper type (17)]
>      expected: FAIL

These should also be deleted (check that the test still passes).
Comment on attachment 8866980 [details]
Bug 1270740 - remove requestAutocomplete DOM code.

https://reviewboard.mozilla.org/r/138578/#review141916
Attachment #8866980 - Flags: review?(bugs) → review+
Comment on attachment 8866979 [details]
Bug 1270740 - Remove requestAutocomplete from toolkit/.

https://reviewboard.mozilla.org/r/138576/#review142220

::: browser/base/content/test/static/browser_misused_characters_in_strings.js
(Diff revision 3)
> -    // Some files, such as requestAutocomplete.dtd, have no entities defined.
>      return;

I only wanted you to remove the part I quoted:
", such as requestAutocomplete.dtd,"
Attachment #8866979 - Flags: review?(MattN+bmo)
Attachment #8866980 - Attachment is obsolete: true
Comment on attachment 8866979 [details]
Bug 1270740 - Remove requestAutocomplete from toolkit/.

https://reviewboard.mozilla.org/r/138576/#review142230
Attachment #8866979 - Flags: review?(MattN+bmo) → review+
Attachment #8867369 - Attachment is obsolete: true
Attachment #8867369 - Flags: review?(bugs)
Attachment #8866980 - Attachment is obsolete: false
https://hg.mozilla.org/mozilla-central/rev/da71eb711143
https://hg.mozilla.org/mozilla-central/rev/5a223dacef72
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Keywords: dev-doc-needed
I've documented this, first by marking the ref doc entry as obsolete:

https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement#Methods

Second by adding a note to the Fx55 ref docs:

https://developer.mozilla.org/en-US/Firefox/Releases/55#APIs_2

(We never wrote a proper ref doc to cover the method in more detail, or documented AutocompleteErrorEvent)
Keywords: dev-doc-needed → dev-doc-complete
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.