Closed Bug 1266495 Opened 8 years ago Closed 7 years ago

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


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




Tracking Status
firefox56 --- fixed


(Reporter: d, Assigned: hsivonen)


(Blocks 2 open bugs)


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


(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 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
Ever confirmed: true
Let's get Henri's opinion here.
Flags: needinfo?(hsivonen)
Whiteboard: btpp-fixlater
See Also: → 616027
Repeating what I said in
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
Flags: needinfo?(hsivonen)
Whiteboard: btpp-fixlater → btpp-active
(FWIW, stuff like 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 Note that per and they also removed the corresponding support for <input name=isindex> as part of We should probably do the same.
Based on Henri's confirmation we've gone ahead and removed it from HTML: Three out of four browsers ought to do it.
Attached patch WIP (lacks test case changes) (obsolete) — Splinter Review
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. :-(
Depends on: 1356181
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>.
Blocks: 1347643
Attachment #8748742 - Attachment is obsolete: true
Attachment #8883503 - Flags: review?(wchen)
Attached patch htmlparser patchSplinter Review
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>.
Attachment #8883503 - Flags: review?(wchen) → review+
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
I've clearly marked it as obsolete on its reference page:

And added a note to the Fx56 rel notes:

Let me know if these sound OK. Thanks!
You need to log in before you can comment on or make changes to this bug.