Site tries to open external URL in new window resulting in blank page

RESOLVED FIXED in Firefox 58

Status

()

P2
normal
RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: ilgaz, Assigned: esawin)

Tracking

Firefox 57
Firefox 58
Unspecified
Android
Points:
---
Bug Flags:
webcompat ?

Firefox Tracking Flags

(fennec+, firefox56 wontfix, firefox57 wontfix, firefox58 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

a year ago
User Agent: Mozilla/5.0 (Android 5.1.1; Mobile; rv:57.0) Gecko/57.0 Firefox/57.0
Build ID: 20170813100228

Steps to reproduce:

1) navigate to https://eksisozluk.com
2) check a database entry like https://eksisozluk.com/entry/3871726
3) tap on a external URL in the entry (this one includes http://www.mozilla.org/products/firefox link)



Actual results:

A blank new tab opens


Expected results:

New tab with external URL should open
(Reporter)

Comment 1

a year ago
I have also tested it on nightly Android version, it exists.
I can reproduce this. But I can not create a minimal example of it for testing. Its working in all other places. I think its a issue with the site event listener.
(Reporter)

Updated

a year ago
OS: Unspecified → Android
Version: 56 Branch → Firefox 57

Comment 3

a year ago
The issue is reproducible on the latest Nightly 58 build 2017-10-30.
Device: Huawei P9 Lite (Android 6.0)
Status: UNCONFIRMED → NEW
tracking-fennec: --- → ?
status-firefox56: --- → affected
status-firefox57: --- → affected
status-firefox58: --- → affected
Ever confirmed: true
Assignee: nobody → esawin
tracking-fennec: ? → +
Priority: -- → P2
(Assignee)

Comment 4

a year ago
That's a regression on Fennec 53 from

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a77180ae4e06fe05353d97beaa750ec65c3ce2d6&tochange=9211bfa02ed5d2807fba8253cff6eb9383ce3f5d

Ben, the interval code has gone through a lot of iterations since, do you have an idea how that change could affect this case?
Flags: needinfo?(bkelly)
There are some sites that try to do this:

1. open a blank window
2. perform some async operation using setTimeout() (usually for tracking)
3. navigator the window opened in (1)

But, opening a new tab puts the current tab in the background.  Which means each setTimeout() in (2) gets throttled and delayed up to a second.

I just tested this on my phone.  If I navigate back to the original page then the new tab is loaded.

Also, I opened the site in desktop edge.  It noticeably pauses for 1 second before navigating the window.  It is almost certainly hitting the same background throttling there.

I believe fennec is havng a much slower experience because mobile.js specifically sets the background delay to 15 minutes:

https://searchfox.org/mozilla-central/source/mobile/android/app/mobile.js#714

This was done in bug 736602.

In general, though, its bad site design to require a background tab to performing timing operations that block navigation of a foreground tab.  There are plenty of ways to avoid this.
Flags: needinfo?(bkelly)
Note, if I manually set dom.min_background_timeout_value back to the default of 1000 the page does load, but with some delay.
(Assignee)

Comment 7

a year ago
Thanks, Ben.

I think the only way to help with such a case on the client side is to delay setTimeout-throttling when backgrounding the tab since reducing the timeout value is out of question.
However, I agree that this is a web-compat issue, we shouldn't have to work around this.
Has Regression Range: --- → yes
Has STR: --- → yes
Flags: webcompat?
(In reply to Eugen Sawin [:esawin] from comment #7)
> I think the only way to help with such a case on the client side is to delay
> setTimeout-throttling when backgrounding the tab since reducing the timeout
> value is out of question.

Yea, I explicitly don't want to do that.  And at least one other browser (edge) does not do that either.

> However, I agree that this is a web-compat issue, we shouldn't have to work
> around this.

Note, we could probably reduce our 15 minute background throttle on mobile now that we have our budget-based throttling in place.  That would at least turn this into "opens slowly" instead of "appears to never open".
(Assignee)

Comment 9

a year ago
(In reply to Ben Kelly [:bkelly] from comment #8)
> Note, we could probably reduce our 15 minute background throttle on mobile
> now that we have our budget-based throttling in place.  That would at least
> turn this into "opens slowly" instead of "appears to never open".

How does the budget-based throttling work in detail? Wouldn't reducing the timeout to anything sensible for this case (~1s) make us vulnerable to e.g., battery-draining scripts running in background?
Andreas can explain it better than me, but basically we enforce a budget on CPU usage in background tabs.  If a setTimeout() callback consumes a lot of time then we dynamically throttle the next callback to fire much later.  The goal is to limit background tabs to at most 1% of CPU or something.

Its currently on by default starting in FF58:

https://searchfox.org/mozilla-central/source/modules/libpref/init/all.js#1319
(Assignee)

Comment 11

a year ago
Keeping the radio active by periodic data requests doesn't need to consume much CPU to quickly drain the battery, do (/can?) we enforce a budget on that, too?
(In reply to Eugen Sawin [:esawin] from comment #11)
> Keeping the radio active by periodic data requests doesn't need to consume
> much CPU to quickly drain the battery, do (/can?) we enforce a budget on
> that, too?

Not that I am aware of, no.
(Assignee)

Comment 13

a year ago
In this case, a periodic 10s network ping will probably not hit its CPU budget but could potentially keep the radio up constantly (radio control state transitions are network-configured and a 10s power-down delay would not be unrealistic for a HSPA+ network).

However, the same issue holds for the active tab, where we don't enforce any limits, so I'm not sure what a good balance to counteract setTimeout abusers would be.
I think what you are talking about is a different feature from the background setTimeout() throttling.  For example, ss designed, we don't throttle at all if the background tab is using WebSockets.  A WebSocket will definitely keep the radio.

If you want something to block network activity on background tabs I think you want to build a different thing.
(Assignee)

Comment 15

a year ago
Created attachment 8924746 [details] [diff] [review]
0001-Bug-1389926-1.0-Reduce-timeout-clamp-for-background-.patch

In this case and assuming that budget-based throttling is in effect on mobile, it might be good to keep parity on the timeouts with desktop to reduce web-compat issues.
Attachment #8924746 - Flags: review?(snorp)
(In reply to Eugen Sawin [:esawin] from comment #15)
> Created attachment 8924746 [details] [diff] [review]
> 0001-Bug-1389926-1.0-Reduce-timeout-clamp-for-background-.patch
> 
> In this case and assuming that budget-based throttling is in effect on
> mobile, it might be good to keep parity on the timeouts with desktop to
> reduce web-compat issues.

If this change in itself is too radical, you now have the opportunity to tweak how the background throttling of timeouts happen with regard to budget! The prefs are:

* dom.timeout.background_throttling_max_budget

This is how large the max budget is, default is 50ms. This budget is kept by every window (we usually say tab, because of this happening for background, but it really is on the window level, not the tab level). Whenever a timeout callback executes, the execution time is deducted from the budget and when the budget is negative, the next timeout cannot trigger until the budget has regenerated.

* dom.timeout.background_budget_regeneration_rate

This determines how many milliseconds it takes to regenerate one millisecond of budget. Default is 100 ms (so, for example, if a budget is overdrawn and becomes -13 ms, it takes 1.3 seconds with the default settings to have a positive budget again).

* dom.timeout.budget_throttling_max_delay

This is the maximum amount of time we'll delay a timeout due to the budget being negative, which really implies that there is a clamp on how small the budget can be as well. This is set to default to 15000 seconds, and this with a regeneration factor of 1/100 ms means that the lowest value that the internal budget of a window can have is -150ms.

The intention with the budget throttling system is mainly to punish timeouts that run "too long and too often". For example, since we, by default, regenerate 10 ms per 1000ms, it is possible to run two timeouts that take 5ms every 500ms, and not overdraw the budget. At this level we'll hit the ordinary, once-per-second, background throttling before the budget throttling. If we want to throttle harder, we can lower the regeneration to, again as an example only, to 5 ms per second (thedom.timeout.background_budget_regeneration_rate set to 200). Then if we try to run 5ms every 500ms, we'll get to the point where the most recent timeout will have resulted in the budget becoming -5ms, and since this takes 1 second to regenerate, we'll not run again until after 1 second, at which point we'll again overdraw by 5ms, and again have to wait 1 second. So as you see, if a page behaves nicely, it still gets to run quite a few timeouts in the background.

The default values are the way they are due to the fact that we wish to achieve a throttling where a window in the background only uses 1% CPU, so doing the maths with the prefs we see that:

1) We assume that the value 50ms is reasonable estimate for how long a timeout callback should be allowed to be without being considered as a "bad" callback, for example the requestIdleCallback spec uses this value for the maximum idle period length.
2) If we assume all callbacks are "good" callbacks then we know that we'll not at most have a negative budget of -50ms until we start throttling.
3) If we want to make background windows only use 1%CPU, then the regeneration rate should regenerate 50ms in 50ms/0.01 = 5000ms, which happens to be 1ms every 100ms

The same kind of calculations can of course be done to achieve different CPU utilization levels.
Attachment #8924746 - Flags: review?(snorp) → review+

Comment 17

a year ago
Pushed by esawin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/24d704a58bca
[1.0] Reduce timeout clamp for background tabs to 1s. r=snorp
https://hg.mozilla.org/mozilla-central/rev/24d704a58bca
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox58: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
status-firefox56: affected → wontfix
status-firefox57: affected → wontfix
You need to log in before you can comment on or make changes to this bug.