Opened 22 months ago

Closed 21 months ago

Last modified 13 months ago

#18914 closed defect (fixed)

Consider removing <isindex>

Reported by: mcs Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: ff45-esr, TorBrowserTeam201605R, tbb-fingerprinting
Cc: brade, gk Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Mozilla is thinking about removing support for <isindex> HTML element. References:

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/isindex
https://groups.google.com/forum/#!topic/mozilla.dev.platform/DV3YBf7wI3M and
https://bugzilla.mozilla.org/show_bug.cgi?id=1266495

The reason we might want to do this for TB 6.0 is that <isindex> generates a form that has a label that contains text that comes from the browser's UI locale (thus leaking that information).

There is a risk that some sites are using this tag.

Child Tickets

Attachments (1)

isindex.html (145 bytes) - added by mcs 22 months ago.
sample page that uses <isindex>

Download all attachments as: .zip

Change History (10)

Changed 22 months ago by mcs

Attachment: isindex.html added

sample page that uses <isindex>

comment:1 Changed 22 months ago by gk

Cc: gk added

I am a bit worried as Ehsan about possible breakage. I wonder whether we could write a patch instead and wait what Mozilla is doing/finding out?

comment:2 Changed 22 months ago by gk

Keywords: TorBrowserTeam201605 added

Dragging into May to have it on our 6.0 radar.

comment:3 Changed 21 months ago by arthuredelstein

Status: newneeds_review

Here's a patch that uses an English-only label on <isindex/> tags. The localized tag is removed. This provides an easy fix while we wait for Mozilla to remove the <isindex> support altogether.

https://github.com/arthuredelstein/tor-browser/commit/18914+1
Hash 018cc9788c202df10a9f6aceaac12af12bd672b6

comment:4 Changed 21 months ago by arthuredelstein

Keywords: TorBrowserTeam201605R added; TorBrowserTeam201605 removed

comment:5 in reply to:  3 ; Changed 21 months ago by mcs

Replying to arthuredelstein:

Here's a patch that uses an English-only label on <isindex/> tags. The localized tag is removed. This provides an easy fix while we wait for Mozilla to remove the <isindex> support altogether.

https://github.com/arthuredelstein/tor-browser/commit/18914+1
Hash 018cc9788c202df10a9f6aceaac12af12bd672b6

This is a good solution.
Is it safe to use u"..." string literals for all compilers?
Mozilla code usually uses NS_LITERAL_STRING.
Otherwise, the changes look good.

comment:6 in reply to:  5 Changed 21 months ago by arthuredelstein

Replying to mcs:

Replying to arthuredelstein:

Here's a patch that uses an English-only label on <isindex/> tags. The localized tag is removed. This provides an easy fix while we wait for Mozilla to remove the <isindex> support altogether.

https://github.com/arthuredelstein/tor-browser/commit/18914+1
Hash 018cc9788c202df10a9f6aceaac12af12bd672b6

This is a good solution.
Is it safe to use u"..." string literals for all compilers?
Mozilla code usually uses NS_LITERAL_STRING.
Otherwise, the changes look good.

Good point. Here's a revised version using NS_LITERAL_STRING.

https://github.com/arthuredelstein/tor-browser/commit/18914+2
Hash 3c2c77205ee2bf92abd975fbed310cfcd57e74dc

comment:7 Changed 21 months ago by mcs

r=brade, r=mcs
This looks good.

comment:8 Changed 21 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Picking this as well for 6.0 as the change seems fairly small and well understood. commit 4166668b7cf72bdb0666e53ecd1abdc41de90c23 on tor-browser-45.1.0esr-6.0-1 has it.

comment:9 Changed 13 months ago by arthuredelstein

Keywords: tbb-fingerprinting added
Note: See TracTickets for help on using tickets.