Description

Using a non-en-US locale does not show the "request English language web pages"-prompt anymore with the switch to 7.0a3. I have not looked closer but I guess the progress listener is broken due to e10s.

Why is this prompt at all? Shouldn't it ask the opposite "Do you want to request web pages of your locale (reduces antifingerprinting protection)?" if should?
(Never seen that switching GUI language affects some program's behavior.)

Why is the delay before showing the prompt only imposed for about:tor?

The old code only showed the prompt when the user accessed an HTTP page. The new code will show it at startup on about:tor, which seems like a less friendly "first run" experience (mostly because the prompt will cover up a lot of the about:tor content). Are we okay with this change in behavior?

Why is the delay before showing the prompt only imposed for about:tor?

The old code only showed the prompt when the user accessed an HTTP page. The new code will show it at startup on about:tor, which seems like a less friendly "first run" experience (mostly because the prompt will cover up a lot of the about:tor content). Are we okay with this change in behavior?

No. I think we should try to get it going the way it was implemented in #18019. Or at least the patch should result in a similar experience.

Makes sense to me. Here's a different patch that I think preserves the behavior from before. One problem is that with e10s, the nsIWebProgressListener no longer delays the request. But "http-on-modify-request" observers correctly delays the request until their observe() function returns, so I am using one of those instead.

The only drawback I see with this approach is that sometimes Firefox makes a background http request (such as checking for updates) and I'm not sure whether or not we want to show the prompt when that happens, and what the criteria should be for ignoring such requests.

Makes sense to me. Here's a different patch that I think preserves the behavior from before. One problem is that with e10s, the nsIWebProgressListener no longer delays the request. But "http-on-modify-request" observers correctly delays the request until their observe() function returns, so I am using one of those instead.

Sounds like a good idea. While testing this for a while I found two possible outcomes both of them being suboptimal:

1) The dialog does not get shown at all. This happens when the Torbutton update check is the first request captured by the observer. Subsequent surfing to different websites does not bring the dialog up either. Not sure why that is happening.

2) I get the dialog shown with the about:tor page (triggered by the updater check). This is pretty confusing as it boils down to a modal dialog during the start-up process, a thing we should avoid if possible.

I think what we could do is to use the setTimeout() call which we need for the homepage case anyway and apply it generally. There should not be any automatic requests on start-up besides the update checks for extensions and the browser itself and the Torbutton version check. As those happen often on start-up and there is no need to show localized web content in a response basically exempting those requests seems fine to me.

The only drawback I see with this approach is that sometimes Firefox makes a background http request (such as checking for updates) and I'm not sure whether or not we want to show the prompt when that happens, and what the criteria should be for ignoring such requests.

The prompt is shown once. And it is not really important for background update requests/checks as the response content displayed in the browser (if it is displayed at all) is not dependent on the Accept-Language header. So it seems fine to me we can ignore this kind of requests.

Thanks for the review and testing. Here is a revised branch, with the same patch revised to trigger the prompt only when a request is launched from a content "load context". I tested this new version with update requests and various about: pages and the prompt was not triggered.

I also noticed that some homepage URLs were not being properly detected by function torbutton_is_homepage_url(aURI). So I added a second patch to this new branch to try to take care of that problem.