Domenic Denicola Reporter
	Description • 8 years ago

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160407164938

Steps to reproduce:

Per https://github.com/whatwg/html/issues/1088 we are discussing removing <isindex> support from the HTML Standard, given Blink and EdgeHTML's successful removals already. Would Gecko be interested in following along?

	Andrew Overholt [:overholt]
	Comment 1 • 8 years ago

Let's get Henri's opinion here.

	Karl Dubost💡 :karlcow
	Comment 2 • 8 years ago

Repeating what I said in https://github.com/w3c/html/issues/240
No issue from a Web Compatibility point of view.

	Henri Sivonen (:hsivonen) Assignee
	Comment 3 • 8 years ago

Let's remove it.

It's a bit sad to give up support for historical HTML, but this feature is pretty annoying implementation-wise.

Also, the removal means there's one fewer locale leaks for Tor Browser to deal with.

	Henri Sivonen (:hsivonen) Assignee
	Comment 4 • 8 years ago

(FWIW, stuff like https://groups.google.com/a/chromium.org/d/msg/blink-dev/14q_I06gwg8/52oBtr2VCAAJ was the reason why I wasn't eager to be on the front lines of removing this.)

Domenic, to save me the trouble of searching Blink sources:
What exactly does "removing" mean? Making the parser treat <isindex> as completely unknown or still treating it as a void element like <bgsound>?

	Anne (:annevk)
	Comment 5 • 8 years ago

Completely unknown per https://codereview.chromium.org/96653004/patch/90001/100050. Note that per https://codereview.chromium.org/96653004/patch/90001/100047 and https://codereview.chromium.org/96653004/patch/90001/100052 they also removed the corresponding support for <input name=isindex> as part of https://codereview.chromium.org/96653004/. We should probably do the same.

	Anne (:annevk)
	Comment 6 • 8 years ago

Based on Henri's confirmation we've gone ahead and removed it from HTML: https://github.com/whatwg/html/commit/5c44abc734eb483f9a7ec79da5844d2fe63d9c3b. Three out of four browsers ought to do it.

	Kohei Yoshino [:kohei]
	Comment 7 • 8 years ago

Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/isindex-support-will-be-removed/

	Simon Pieters [:zcorpan]
	Comment 9 • 8 years ago

Tests added to web-platform-tests in

https://github.com/w3c/web-platform-tests/pull/3684

and html5lib-tests in

https://github.com/html5lib/html5lib-tests/pull/74

	Mike Taylor [:miketaylr]
	Comment 10 • 8 years ago

Dropped in the latest Safari Tech Preview: https://developer.apple.com/safari/technology-preview/release-notes/

	Henri Sivonen (:hsivonen) Assignee
	Comment 11 • 8 years ago

ehsan, I didn't get around to adding telemetry the last time round this was discussed. Given comment 10, are you OK with proceeding without telemetry? We're now the last major engine to remove this.

	Henri Sivonen (:hsivonen) Assignee
	Comment 12 • 8 years ago

And actually neeinfoing ehsan now. ehsan, please see the previous comment.

	(no longer active)
	Comment 13 • 8 years ago

No, please add a telemetry probe first.

The last time we removed a Gecko only feature (remote jar: URLs, bug 1215235) we ended up breaking IBM iNotes and had to do a quick dot release (bug 1255139.)  It really isn't worth it having to go through another one of those firedrills and lose more users over it.

	Henri Sivonen (:hsivonen) Assignee
	Comment 14 • 7 years ago

And now not having removed this already is getting in the way of fixing bug 1355479. :-(

	Henri Sivonen (:hsivonen) Assignee
	Comment 15 • 7 years ago

The telemetry dashboard for release 54 says 36.15 million sessions without isindex submission versus 8 (just 8, no multiplier) sessions with at least one isindex submission.

I think it's safe to conclude that we can remove <isindex>.

	Henri Sivonen (:hsivonen) Assignee
	Comment 19 • 7 years ago

Attaching the htmlparser patch, too, to make it easier to land this (if needed) while I'm away from Bugzilla.

	William Chen [:wchen]
	Comment 20 • 7 years ago
mozreview-review

Comment on attachment 8883503 [details]
Bug 1266495 - Remove <isindex>.

https://reviewboard.mozilla.org/r/154420/#review161024

	Pulsebot
	Comment 21 • 7 years ago

Pushed by hsivonen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f999b725ef69
Remove <isindex>. r=wchen

	Kohei Yoshino [:kohei]
	Comment 22 • 7 years ago

Posted the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2017/isindex-support-has-been-dropped/

	Henri Sivonen (:hsivonen) Assignee
	Comment 23 • 7 years ago

https://hg.mozilla.org/projects/htmlparser/rev/40a176e227d284d4ce8b66dd5fe4577860f5f563
Mozilla bug 1266495 - Remove <isindex> from the C++ side, too. r=wchen.

	Ryan VanderMeulen [:RyanVM]
	Comment 24 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/f999b725ef69

	Chris Mills [:cmills]
	Comment 25 • 7 years ago

I've clearly marked it as obsolete on its reference page:

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/isindex

And added a note to the Fx56 rel notes:

https://developer.mozilla.org/en-US/Firefox/Releases/56#Removals_from_the_web_platform

Let me know if these sound OK. Thanks!