Closed Bug 1266495 Opened 9 years ago Closed 7 years ago

Consider removing <isindex> from the parser and form submission [tor 18914]

Categories

(Core :: DOM: HTML Parser, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: d, Assigned: hsivonen)

References

(Blocks 2 open bugs)

Details

(Keywords: dev-doc-complete, site-compat, Whiteboard: btpp-active [tor][fingerprinting])

Attachments

(2 files, 1 obsolete file)

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?
OS: Unspecified → All
Hardware: Unspecified → All
Status: UNCONFIRMED → NEW
Ever confirmed: true
Let's get Henri's opinion here.
Flags: needinfo?(hsivonen)
Whiteboard: btpp-fixlater
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
Flags: needinfo?(hsivonen)
Whiteboard: btpp-fixlater → btpp-active
(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.
Whiteboard: btpp-active → btpp-active [tor][tor 18914]
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.
Flags: needinfo?(ehsan)
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.
Flags: needinfo?(ehsan)
Blocks: 1330892
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>.
Attached patch htmlparser patch (deleted) — Splinter Review
Attaching the htmlparser patch, too, to make it easier to land this (if needed) while I'm away from Bugzilla.
Attachment #8883503 - Flags: review?(wchen) → review+
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
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.

Attachment

General

Created:
Updated:
Size: