Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop IMG element from form-associated elements #294

Closed
tkent-google opened this issue Oct 29, 2015 · 18 comments

Comments

@tkent-google
Copy link
Collaborator

commented Oct 29, 2015

Proposal: Remove 'img' from the element list on https://html.spec.whatwg.org/multipage/forms.html#form-associated-element

Reason:
I have no idea of use-cases, and there is no usage according to statistics of Google Chrome.
https://www.chromestatus.com/metrics/feature/timeline/popularity/246

@foolip

This comment has been minimized.

Copy link
Member

commented Oct 29, 2015

The spec has two concepts, the form-associated elements and the subcategory listed elements. The img element is a form-associated element, but not in any of the subcategories.

Per spec, HTMLFormControlsCollection (form.elements['foo']) uses listed elements, while the indexed for named property retrieval (form['foo']) uses first the listed elements, and then falls back to img elements and the past names map.

Here are two ad-hoc test for these two code paths:
http://software.hixie.ch/utilities/js/live-dom-viewer/saved/3716

In Blink, both code paths return an image element. In Gecko and Edge, only the form['foo'] variant does. The spec agrees with Gecko and Edge.

Blink's form['foo'] is implemented in terms of the elements collection internally, with only the past names fallback. If the img fallback were moved out to the same place, that would align more with the spec and other engines.

What do you think, @tkent-google?

@tkent-google

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 29, 2015

Blink's form['foo'] is implemented in terms of the elements collection internally, with only the past names fallback. If the img fallback were moved out to the same place, that would align more with the spec and other engines.

It doesn't matter. The proposal is that neither form.elements.foo nor form.foo returns img elements because no one uses such behavior. https://www.chromestatus.com/metrics/feature/timeline/popularity/246 contains both cases.

@zcorpan

This comment has been minimized.

Copy link
Member

commented Oct 29, 2015

SGTM. @bzbarsky @travisleithead does this seem OK for you to stop returning img for form.foo?

@zcorpan

This comment has been minimized.

Copy link
Member

commented Oct 29, 2015

cc @hober for WebKit

@bzbarsky

This comment has been minimized.

Copy link
Collaborator

commented Oct 29, 2015

If we have actual data showing that no one is using the form.foo version, I would probably be fine with removing it.

zcorpan added a commit that referenced this issue Oct 30, 2015
Apparently nobody relies on `form.foo` returning `img` elements.
https://www.chromestatus.com/metrics/feature/timeline/popularity/246

Remove `img` from "form-associated element" category which makes it
no longer be associated with a `form` in the parser.

Remove the `img` step from `form`'s "indexed for named property
retrieval" algorithm.
@tkent-google

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 2, 2015

The main motivation of this proposal is to reduce maintenance burden.

If we follow the current specification, implementation would be simple, and would have no maintenance burden. However, the current specification is not accurate.
The specification says:

https://html.spec.whatwg.org/multipage/forms.html#dom-form-nameditem

2 If candidates is empty, let candidates be a live RadioNodeList object containing all the img elements that are descendants of the form element and that have either an id attribute or a name attribute equal to name, in tree order.

Major four browsers don't work so. They return img element of which form owner is the form element. It means img elements support non-ancester form-association by HTML parser like the following:

<body>
<table><form></table>
<img name=i>
</body>

So, img elements need to keep track of their form owner, but the logic is slightly different from other form control elements because of the difference between 'form-associated elements' and 'reassociateable elements'

I know both of Blink and WebKit still have implementation bugs of form-association. We can fix the implementation, but removing the feature makes much sense because of zero usage.

@foolip

This comment has been minimized.

Copy link
Member

commented Nov 2, 2015

Here's that case as a test, where Blink, Gecko and Edge all return the img element:
http://software.hixie.ch/utilities/js/live-dom-viewer/saved/3725

I now see that there's a bunch of extra code to handle this case in Blink in both HTMLFormElement and HTMLImageElement.

I will support making the change in Blink, and to change the spec once that's reached Chrome stable without incident.

@zcorpan

This comment has been minimized.

Copy link
Member

commented Dec 14, 2015

Update on blink-dev:

The correct UseCounter was shipped with Google Chrome 47, and the highest
count was 0.0036%.
https://www.chromestatus.com/metrics/feature/timeline/popularity/246

It's not safe to remove it silently. So, my plan is:

  • Deprecate it since Google Chrome 49
  • Google Chrome 51 removes it.
@foolip

This comment has been minimized.

Copy link
Member

commented Dec 14, 2015

I've followed up on blink-dev, have some questions about proceeding with this given that usage is higher than we thought.

@tkent-google

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 16, 2015

I decided not to remove the feature. I'll close this issue.

I'll investigate whether we can remove non-descendant IMG-FORM association.

zcorpan added a commit that referenced this issue Mar 2, 2016
Use counters indicate non-zero usage, so changing implementations
is more risk than it's worth.

Previous issue: #294
Previous PR: #297
@zcorpan

This comment has been minimized.

Copy link
Member

commented Mar 2, 2016

FYI the spec now requires including non-descendant form-associated imgs, see #778

@TimvdLippe

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2018

Sorry for posting on this old issue, but I wanted to comment here before opening a new issue. Today I was working on some compatibility in a polyfill and came across the "form-associated" elements. Per the specification, I included img in my polyfill. However, later I discovered that img should not be included in this list (per https://github.com/w3c/web-platform-tests/blob/e4efc10a82630de47f920a737aa8ea94845191af/html/resources/common.js#L33-L35)

Now my question is, should the list stated in https://html.spec.whatwg.org/multipage/forms.html#form-associated-element remove the reference to img? Because per https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#form-owner all of these elements must have the form attribute/property, but in Chrome and Firefox (browser I could test in) the following snippet returns undefined:

Object.getOwnPropertyDescriptor(window.HTMLImageElement.prototype, 'form')
@bzbarsky

This comment has been minimized.

Copy link
Collaborator

commented Apr 2, 2018

@TimvdLippe An <img> is a form-accociated element, and has a "form owner". But it does not have a form property that allows script access to that "form owner". The existence of the "form owner" is independent of whether there's a form property.

In particular, only "listed elements" in the sense of https://html.spec.whatwg.org/multipage/forms.html#category-listed have a form property and allow an attribute named "form" to change what the form owner is. And <img> is not in that list.

So that's the current state of things in the spec and at least in the Firefox implementation: <img> is form-associated, but not a "listed element".

@bzbarsky

This comment has been minimized.

Copy link
Collaborator

commented Apr 2, 2018

Oh, and the reason <img> is form-associated is because this:

<form><img name="foo"></form>
<script>console.log(document.querySelector("form").foo)</script>

logs the <img> element.

@domenic

This comment has been minimized.

Copy link
Member

commented Apr 2, 2018

@TimvdLippe it'd probably be a good idea to fix that test file to have the correct list of form-associated elements. Although you'd also have to check what other test files use it; perhaps some of them are not actually testing form-associatedness, but instead listed-ness. Perhaps file a bug on the web-platform-tests repo if you're not interested in doing that legwork :).

@bzbarsky

This comment has been minimized.

Copy link
Collaborator

commented Apr 2, 2018

Looks like that list is only used in the tests in shadow-dom/untriaged/html-elements-in-shadow-trees/html-forms

Some of those tests don't care about what sort of element they're testing with, afaict (so not sure why they restrict to this list), while others are definitely trying to test listedness of elements. There isn't a single test in there that actually tests form-associatedness via the named getter, afaict (and there probably should be).

@TimvdLippe

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2018

@bzbarsky Thanks for the quicky reply and detailed explanation. It is a lot clearer to me now. Apparently there is more confusion about associated-ness vs listed-ness in the WPT tests, so maybe this can be clarified in someway. Anyways, my problem has been resolved 🎉

@TimvdLippe

This comment has been minimized.

Copy link
Contributor

commented Apr 14, 2018

I have opened web-platform-tests/wpt#10471 that someone else can resolve, as I think it is a good first issue for any new contributor. Once again thanks everyone for the clarification and quick help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
You can’t perform that action at this time.