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)
Core
DOM: HTML Parser
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)
59 bytes,
text/x-review-board-request
|
wchen
:
review+
|
Details |
24.21 KB,
patch
|
Details | Diff | Splinter Review |
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?
Reporter | ||
Updated•9 years ago
|
OS: Unspecified → All
Hardware: Unspecified → All
Updated•9 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 1•9 years ago
|
||
Let's get Henri's opinion here.
Flags: needinfo?(hsivonen)
Whiteboard: btpp-fixlater
Updated•9 years ago
|
See Also: → https://github.com/w3c/html/issues/240
Comment 2•9 years ago
|
||
Repeating what I said in https://github.com/w3c/html/issues/240
No issue from a Web Compatibility point of view.
Assignee | ||
Comment 3•9 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.
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
Flags: needinfo?(hsivonen)
Updated•9 years ago
|
Whiteboard: btpp-fixlater → btpp-active
Assignee | ||
Comment 4•9 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>?
Comment 5•9 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.
Comment 6•9 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.
Comment 7•9 years ago
|
||
Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/isindex-support-will-be-removed/
Keywords: dev-doc-needed,
site-compat
Assignee | ||
Comment 8•9 years ago
|
||
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
Updated•8 years ago
|
Whiteboard: btpp-active → btpp-active [tor][tor 18914]
Comment 10•8 years ago
|
||
Dropped in the latest Safari Tech Preview: https://developer.apple.com/safari/technology-preview/release-notes/
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.
Assignee | ||
Comment 12•8 years ago
|
||
And actually neeinfoing ehsan now. ehsan, please see the previous comment.
Flags: needinfo?(ehsan)
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.
Flags: needinfo?(ehsan)
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]
Assignee | ||
Comment 14•8 years ago
|
||
And now not having removed this already is getting in the way of fixing bug 1355479. :-(
Updated•8 years ago
|
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>.
Assignee | ||
Updated•7 years ago
|
Attachment #8748742 -
Attachment is obsolete: true
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8883503 -
Flags: review?(wchen)
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.
Comment 20•7 years ago
|
||
mozreview-review |
Comment on attachment 8883503 [details]
Bug 1266495 - Remove <isindex>.
https://reviewboard.mozilla.org/r/154420/#review161024
Attachment #8883503 -
Flags: review?(wchen) → review+
Comment 21•7 years ago
|
||
Pushed by hsivonen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f999b725ef69
Remove <isindex>. r=wchen
Comment 22•7 years ago
|
||
Posted the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2017/isindex-support-has-been-dropped/
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.
Comment 24•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
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!
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•