Intent to Remove: img form-association

154 views
Skip to first unread message

TAMURA, Kent

unread,
Nov 1, 2015, 11:02:02 PM11/1/15
to blink-dev

Primary eng (and PM) emails

tk...@chromium.org


Link to “Intent to Deprecate” thread

None. I think we may skip deprecation because of low usage.

If we skip deprecation, I'm going to remove this feature just after M48 branch.


Summary

Drop img element from form-associated elements.

More concretely, remove the following two behaviors:

[1] form.foo returns an img element

[2] form.elements.foo returns an img element


I proposed a specification change, and concluded that we'll change the specification if Blink successfully remove the feature.

https://github.com/whatwg/html/issues/294

https://github.com/whatwg/html/pull/297


[1] https://html.spec.whatwg.org/multipage/forms.html#dom-form-nameditem IE, Firefox, Safari, Google Chrome support this.

[2] Non-standard WebKit quirk. Safari and Google Chrome support it.


Motivation

- This feature is not reasonable at all. Image is not a form control.

- Code maintenance burden. Form-association was a source of crash bugs, and img form-association is slightly different from form control form-association.

See https://github.com/whatwg/html/issues/294#issuecomment-152884108


Compatibility Risk

Low-to-middle.

+ Major browsers support it.

+ This is very long-standing feature. WebKit introduced it 11 years ago.

Note that neither HTML4 nor DOM Level 2 HTML doesn't define it.

+ In the worst case, JavaScript code stops by accessing undefined value.

- Usage is extremely low (See below).



Usage information from UseCounter

https://www.chromestatus.com/metrics/feature/timeline/popularity/246


OWP launch tracking bug

crbug.com/550252


Entry on the feature dashboard

https://www.chromestatus.com/feature/5725368270979072


--
TAMURA Kent
Software Engineer, Google


Philip Jägenstedt

unread,
Nov 2, 2015, 8:34:12 AM11/2/15
to TAMURA, Kent, blink-dev
LGTM1

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

Rick Byers

unread,
Nov 2, 2015, 3:13:18 PM11/2/15
to Philip Jägenstedt, TAMURA, Kent, blink-dev
I double checked the internal usage - it's effectively 0 (but just enough above 0 to show that the UseCounter is indeed working <grin>).  Given that, LGTM2.  If the usage had been "low" but non-zero, I probably would have argued for a deprecation period.

Rick

Chris Harrelson

unread,
Nov 3, 2015, 5:19:23 PM11/3/15
to Rick Byers, Philip Jägenstedt, TAMURA, Kent, blink-dev
LGTM3.

Daniel Bratell

unread,
Nov 4, 2015, 11:01:45 AM11/4/15
to Philip Jägenstedt, Rick Byers, TAMURA, Kent, blink-dev
On Mon, 02 Nov 2015 21:12:47 +0100, Rick Byers <rby...@chromium.org> wrote:

I double checked the internal usage - it's effectively 0 (but just enough above 0 to show that the UseCounter is indeed working <grin>).  Given that, LGTM2.  If the usage had been "low" but non-zero, I probably would have argued for a deprecation period.

Is it in the forms member array? I just realised that people like to access form elements by number and removing an element will reorder anything later.

Does the usecounter include all the cases when there is an image in the forms member array?

/Daniel

--
/* Opera Software, Linköping, Sweden: CET (UTC+1) */

Philip Jägenstedt

unread,
Nov 5, 2015, 8:34:22 AM11/5/15
to Daniel Bratell, Rick Byers, TAMURA, Kent, blink-dev
On Wed, Nov 4, 2015 at 5:01 PM, Daniel Bratell <bra...@opera.com> wrote:
On Mon, 02 Nov 2015 21:12:47 +0100, Rick Byers <rby...@chromium.org> wrote:

I double checked the internal usage - it's effectively 0 (but just enough above 0 to show that the UseCounter is indeed working <grin>).  Given that, LGTM2.  If the usage had been "low" but non-zero, I probably would have argued for a deprecation period.

Is it in the forms member array? I just realised that people like to access form elements by number and removing an element will reorder anything later.

Fortunately not, the indexed getter ignores the img element:
 
Does the usecounter include all the cases when there is an image in the forms member array?

 It's in the implementation of the HTMLFormControlsCollection named getter, so only the cases where it's actually ever returned, not all cases where it could potentially be returned.

Philip

TAMURA, Kent

unread,
Nov 5, 2015, 6:40:03 PM11/5/15
to Philip Jägenstedt, Daniel Bratell, Rick Byers, blink-dev
Philip is right.
The use counter covers all of user-visible scenarios of img form-association.

TAMURA, Kent

unread,
Nov 13, 2015, 3:15:28 AM11/13/15
to Philip Jägenstedt, Daniel Bratell, Rick Byers, blink-dev
I found the counter was wrong!
I'm fixing it, and will add correct data when it gets ready.


TAMURA, Kent

unread,
Dec 10, 2015, 8:21:49 PM12/10/15
to Philip Jägenstedt, Daniel Bratell, Rick Byers, blink-dev
The correct UseCounter was shipped with Google Chrome 47, and the highest count was 0.0036%.

It's not safe to remove it silently.  So, my plan is:
 - Deprecate it since Google Chrome 49
 - Google Chrome 51 removes it.

Chris Harrelson

unread,
Dec 10, 2015, 11:15:47 PM12/10/15
to TAMURA, Kent, Philip Jägenstedt, Daniel Bratell, Rick Byers, blink-dev
Sounds good to me.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

Philip Jägenstedt

unread,
Dec 14, 2015, 6:07:25 AM12/14/15
to Chris Harrelson, TAMURA, Kent, Daniel Bratell, Rick Byers, blink-dev
The highest count is now 0.0049%, and other counters from M47 have not stabilized yet, even though it's possible they will stay at current levels.

Since this code path apparently is being hit, do you know how pages might depend on it? This is basically impossible to grep for in httparchive, but given the age of this API, I would expect that a lot of it is old content that's not being actively maintained.

As you pointed out on GitHub, the spec for this is simple but wrong, as even img elements that are not descendants of the form element can be returned. Short of ripping all of this out, do you think it's worth refining the use counter to see what amount of content actually depends on this? I would guess is that it's a very small fraction of the existing use counter, in which case simply aligning with the spec would be safe for compat and bring us closer to what Gecko and Edge do.

TAMURA, Kent

unread,
Dec 15, 2015, 1:02:19 AM12/15/15
to Philip Jägenstedt, Chris Harrelson, Daniel Bratell, Rick Byers, blink-dev
This feature has three types of behaviour:
  A) What the specification defines; form.imageName where |imageName| is a descendant of |form|.
  B) What the specification lacks, and all browsers agree; form.imageName where |imageName| is not a descendant of |form| and is associated by HTML parser.
  C) WebKit-only quirk:  form.elements.imageName

The current UseCounter::FormNameAccessForImageElement represents A + B.

How about the following plan?

M49:
  - Add a deprecation message for all of A, B, and C anyway
  - Add UseCounter for B and C.

Before M51 branch:
  If UseCounter.FormNameAccessForImageElement decreases,  we remove the entire feature (A + B + C).
  Otherwise,
    - We withdraw the deprecation.  We don't remove A.
    - If UseCounters for B and C are small enough, we remove them.

Philip Jägenstedt

unread,
Dec 15, 2015, 6:31:32 AM12/15/15
to TAMURA, Kent, Chris Harrelson, Daniel Bratell, Rick Byers, blink-dev
I took a closer look at the use counter and the change that was cherry-picked to M47. AFAICT, the counter originally covered only case C, form.elements.imageName, and then usage was ~0. When it was extended to cover case A+B, form.imageName, usage jumped to current levels.

Given this, we can say that case C, the WebKit-only quirk, would be safe to remove. As for A+B, if parsing things like <table><form></table><img name=x> is the only thing that can trigger case B, I would expect that the per-spec case A accounts for the majority of usage. In other words, "We withdraw the deprecation.  We don't remove A." seems like the mostly likely outcome.

How about, slightly differently:

M49:
  • Add new use counters for cases A, B and C.
  • Deprecate B+C with a M51 target for removal.
Before M51 branch:
  • If everything is as expected, remove B+C and keep only the per-spec behavior A.
  • Otherwise, reconsider.
Unfortunately, removing A+B+C isn't a possible outcome with this approach, but without an idea of how pages depend on this, it's hard to say what the risks are.

What do you think?

PhistucK

unread,
Dec 15, 2015, 12:14:13 PM12/15/15
to Philip Jägenstedt, TAMURA, Kent, Chris Harrelson, Daniel Bratell, Rick Byers, blink-dev
Why are you so worried, given that the use counters are much below the typical deprecation level (0.005 versus 0.02?)? Why not remove all of the cases after a release or two of deprecation warnings?


PhistucK

Rick Byers

unread,
Dec 15, 2015, 1:02:38 PM12/15/15
to PhistucK, Philip Jägenstedt, TAMURA, Kent, Chris Harrelson, Daniel Bratell, blink-dev
FWIW I think the "< 0.02" level is a very coarse and overly optimistic guideline.  If we actually serious break even 0.005% of all page views, that's still a HUGE negative impact on the user experience - causing frustration for many users every single day.  I personally see it more as "If usage is > 0.02% then it's probably not even worth considering deprecation, otherwise it may be worth exploring whether there is a safe path with minimal user impact".

Rick

Philip Jägenstedt

unread,
Dec 15, 2015, 3:05:02 PM12/15/15
to Rick Byers, PhistucK, TAMURA, Kent, Chris Harrelson, Daniel Bratell, blink-dev
Right, when it comes to compat risk, in addition to usage numbers, one might also consider:
  • What percentage is accidentally triggering the counter without using the API? (A problem for IDL attributes and unused CSS.)
  • What percentage would actually break? (Feature detection and fallbacks is more likely the less interoperable the API is.)
  • What is the user-visible breakage?
  • How much work is it to fix the page, and is it even maintained?
For most removals in the 0.01% range, we have good reason that the true breakage is much smaller. If we knew something about the pages triggering the use counter, we would have a better idea about the compat risk here too. Unfortunately I can't see how to do a httparchive search for this. (One could run a instrumented build that crashes on this code path and use that for a few days/weeks until something turns up, though.)

TAMURA, Kent

unread,
Dec 15, 2015, 7:32:36 PM12/15/15
to Philip Jägenstedt, Rick Byers, PhistucK, Chris Harrelson, Daniel Bratell, blink-dev
Ok, I withdraw this intent now.

Probably I didn't send the intent-to-remove if I had the correct usage count.  I tried to remove this long-standing feature because it looked to have no usage.
Philip and Rick are right.  We can't say this removal is safe, and we don't need to take unnecessary risk.  We should focus on user happiness rather than reducing code complexity.

I'll add counters for B and C, and will send new intents if the numbers are ignorable.

AFAICT, the counter originally covered only case C

No.  The old counter counted only |'imageName' in form.elements| and |form.elements.hasOwnProperty('imageName')|.  We have no data for C.

Philip Jägenstedt

unread,
Dec 16, 2015, 10:23:35 AM12/16/15
to TAMURA, Kent, Rick Byers, PhistucK, Chris Harrelson, Daniel Bratell, blink-dev
On Wed, Dec 16, 2015 at 1:32 AM, TAMURA, Kent <tk...@chromium.org> wrote:
> Ok, I withdraw this intent now.
>
> Probably I didn't send the intent-to-remove if I had the correct usage
> count.  I tried to remove this long-standing feature because it looked to
> have no usage.
> Philip and Rick are right.  We can't say this removal is safe, and we don't
> need to take unnecessary risk.  We should focus on user happiness rather
> than reducing code complexity.
>
> I'll add counters for B and C, and will send new intents if the numbers are
> ignorable.
>
>> AFAICT, the counter originally covered only case C
>
> No.  The old counter counted only |'imageName' in form.elements| and
> |form.elements.hasOwnProperty('imageName')|.  We have no data for C.

Oh... this was not at all obvious. The counter was in HTMLFormControlsCollection::namedItem, there's a [ImplementedAs=namedGetter] for namedItem in the IDL that I missed.

I hope that the new use counters confirm that this could all be simplified!

TAMURA, Kent

unread,
Feb 17, 2016, 3:43:30 AM2/17/16
to Philip Jägenstedt, Rick Byers, PhistucK, Chris Harrelson, Daniel Bratell, blink-dev
Google Chrome 48 has counters for B and C.

>  A) What the specification defines; form.imageName where |imageName| is a descendant of |form|.
>  B) What the specification lacks, and all browsers agree; form.imageName where |imageName| is not a descendant of |form| and is associated by HTML parser.
>  C) WebKit-only quirk:  form.elements.imageName

FormNameAccessForImageElement (A+B): 0.0042%
FormNameAccessForNonDescendantImageElement (B): 0.0008%
FormControlsCollectionNameAccessForImageElement (C): 0.0001%
(Note that these values were taken from internal data, and were computed with data for the last one week.)

It seems removing C is ok.  I don't think intent-to-remove is necessary because it is just a bug fix.  The ratio only on Android was lower than 0.0001%.

Removing B is still risky.  19% of form.imageName is B.

Simon Pieters

unread,
Feb 17, 2016, 1:00:02 PM2/17/16
to Philip Jägenstedt, TAMURA, Kent, Rick Byers, PhistucK, Chris Harrelson, Daniel Bratell, blink-dev
On Wed, 17 Feb 2016 09:43:06 +0100, TAMURA, Kent <tk...@chromium.org>
wrote:

> Google Chrome 48 has counters for B and C.
>
>> A) What the specification defines; form.imageName where |imageName|
>> is a
> descendant of |form|.
>> B) What the specification lacks, and all browsers agree;
>> form.imageName
> where |imageName| is not a descendant of |form| and is associated by HTML
> parser.
>> C) WebKit-only quirk: form.elements.imageName
>
> FormNameAccessForImageElement (A+B): 0.0042%
> FormNameAccessForNonDescendantImageElement (B): 0.0008%
> FormControlsCollectionNameAccessForImageElement (C): 0.0001%
> (Note that these values were taken from internal data, and were computed
> with data for the last one week.)
>
> It seems removing C is ok. I don't think intent-to-remove is necessary
> because it is just a bug fix. The ratio only on Android was lower than
> 0.0001%.
>
> Removing B is still risky. 19% of form.imageName is B.

Filed a new spec issue https://github.com/whatwg/html/issues/703

Do I understand correctly that you will not attempt to remove B?

--
Simon Pieters
Opera Software

PhistucK

unread,
Feb 17, 2016, 1:33:23 PM2/17/16
to Simon Pieters, Philip Jägenstedt, TAMURA, Kent, Rick Byers, Chris Harrelson, Daniel Bratell, blink-dev
All of them sound very small and not risky, am I missing something?


PhistucK

Philip Jägenstedt

unread,
Feb 17, 2016, 9:01:34 PM2/17/16
to TAMURA, Kent, Rick Byers, PhistucK, Chris Harrelson, Daniel Bratell, blink-dev
Yeah, removing C ASAP sounds like a good idea. As for B, that is tricky, and IIUC removing that is what would allow for some simplification. 0.0008% is a small number, but we already have basic interop here, so that raises the bar significantly. What might inform the discussion is an example of a site among that 0.0008%. I wouldn't be too surprised if both A and B are hit by accident some of the time, and if returning null instead wouldn't actually break the page. But, this is impossible to grep for, one would need to crawl the web with a custom build.

TAMURA, Kent

unread,
Feb 18, 2016, 9:10:19 PM2/18/16
to PhistucK, Simon Pieters, Philip Jägenstedt, Rick Byers, Chris Harrelson, Daniel Bratell, blink-dev
On Thu, Feb 18, 2016 at 3:32 AM, PhistucK <phis...@gmail.com> wrote:
All of them sound very small and not risky, am I missing something?

Removal risk is evaluate with something like:
    <counter value> * <number of implementations> * <years we have shipped for> * ...

We think the risk is high in this case though the counter values are lower.
Right.
 


--
Simon Pieters
Opera Software

Reply all
Reply to author
Forward
0 new messages