Closed Bug 1266495 Opened 8 years ago Closed 7 years ago
Consider removing <isindex> from the parser and form submission [tor 18914]
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?
Let's get Henri's opinion here.
Repeating what I said in https://github.com/w3c/html/issues/240 No issue from a Web Compatibility point of view.
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.
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
(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>?
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.
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.
Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/isindex-support-will-be-removed/
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
Whiteboard: btpp-active → btpp-active [tor][tor 18914]
Dropped in the latest Safari Tech Preview: https://developer.apple.com/safari/technology-preview/release-notes/
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.
And actually neeinfoing ehsan now. ehsan, please see the previous comment.
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.
Priority: -- → P2
Summary: Consider removing <isindex> from the parser and form submission → Consider removing <isindex> from the parser and form submission [tor 18914]
Whiteboard: btpp-active [tor][tor 18914] → btpp-active [tor][fingerprinting]
And now not having removed this already is getting in the way of fixing bug 1355479. :-(
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>.
Attachment #8748742 - Attachment is obsolete: true
Attachment #8883503 - Flags: review?(wchen)
Attaching the htmlparser patch, too, to make it easier to land this (if needed) while I'm away from Bugzilla.
Comment on attachment 8883503 [details] Bug 1266495 - Remove <isindex>. https://reviewboard.mozilla.org/r/154420/#review161024
Attachment #8883503 - Flags: review?(wchen) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/f999b725ef69 Remove <isindex>. r=wchen
Posted the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2017/isindex-support-has-been-dropped/
https://hg.mozilla.org/projects/htmlparser/rev/40a176e227d284d4ce8b66dd5fe4577860f5f563 Mozilla bug 1266495 - Remove <isindex> from the C++ side, too. r=wchen.
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!
You need to log in before you can comment on or make changes to this bug.