1995636 - Firefox throws from history APIS (e.g. pushState, replaceState) if it's been called rapidly enough to trigger the rate-limit

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Daniel Holbert [:dholbert]]

STR:

1. Load attached testcase.
2. (optional) Check web console.

EXPECTED RESULTS:
500 calls to pushState.
No exception thrown.

ACTUAL RESULTS:
200 calls to pushState.
Exception is thrown (which is why we stop at 200).

This is tracked in https://github.com/livewire/livewire/discussions/7746 . One comment there suggests flagging Firefox as unsupported as a result.

In bug 1993517 comment 6, we've got a bug where a Swedish banking site seems to be blocking Firefox-on-Windows due to this behavior:

  // windows firefox produce recursive navigation error
  // Too many calls to location or history APIs in a short period of time
  // https://github.com/livewire/livewire/discussions/7746
  if (isWindows && isFirefox) {
    return <BankIDViewFirefoxError />;
  }

(Windows is relevant there because the bank seems to optionally use a native Windows app, which they communicate with from the browser somehow; not sure if via a local web server or something else.)

Not sure whether it'd be prudent for us to change behavior here, but I wanted to file this since it's at least a behavioral difference with Chromium that's causing enough web-developer-trouble that Firefox is blocked in at least that one case.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

Comment 2

[Daniel Holbert [:dholbert]]

Safari-on-macOS behaves like Firefox here, with an even stricter threshold - they throw after 100 calls to the history API, in my testcase. (And https://github.com/livewire/livewire/discussions/7746 does mention Safari as being affected, too.)

Chrome seems to be the only engine (among the major 3) to have no limit here to never throw exceptions here.

[Edited to clarify -- it's not that Chrome has no limit, but rather that they just make these history APIs silently-do-nothing when they've reached their limit.]

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Gecko and webkit follow the spec
https://html.spec.whatwg.org/#shared-history-push/replace-state-steps:~:text=Optionally%2C%20throw,succession%2E%29

https://github.com/whatwg/html/issues/11108

Comment 4

[Daniel Holbert [:dholbert]]

Thanks. Looks like Chrome is willing to match the WebKit/Gecko spec-compliant behavior, per that whatwg issue, though they haven't had any recent activity on their bug ( https://issues.chromium.org/issues/40554715 ). Maybe this is a good reason to nudge them to adapt - that will hopefully push sites to handle this better rather than blocking specific browsers as is potentially the case in bug 1993517.

I was wondering how/why sites would even be running up against this - https://github.com/livewire/livewire/discussions/7746#discussioncomment-10389123 describes one scenario:

• "a ParentComponent that loads X amount of ChildComponents" (where X = 100)
• "all of the ChildComponents [store] state in the url search"
• So presumably each ChildComponent uses the history API once or twice to store some data in the location.search object (the string after the ? in the URL); and since there are 100 child-components, that means there are at least 100 such calls; and if these happen in rapid succession and surpass the browser's threshold, then one of the history API invocations will throw (if the browser is Safari or Firefox).
• ...and If I'm understanding https://github.com/whatwg/html/issues/11108 correctly, current Chromium will currently just treat the throttled invocations as no-ops and some of the ChildComponent state will be silently lost instead of stored in the URL.

Comment 5

[Simon Pieters [:zcorpan]]

I commented in the Chromium issue.

Comment 6

[Simon Pieters [:zcorpan]]

We could increase the limit to, say, 1000. Pages can still hit the limit, but it's less likely. The limit was intended to not be reached by accident, but it seems from this issue it has been reached by accident.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

But it does tell the site has a bug. Someone is misusing the API.
We could still use 1000, sure.

Comment 8

[Andreas Farre [:farre]|PTO -> Aug 7th]

Attachment: Bug 1995636 - Increase navigation rate limiting threshold. r=smaug!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/7b62c4b593fd
https://hg.mozilla.org/integration/autoland/rev/5c897160129f
Increase navigation rate limiting threshold. r=smaug

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f613aa6b6140
https://hg.mozilla.org/integration/autoland/rev/ae421aa8ee44
Revert "Bug 1995636 - Increase navigation rate limiting threshold. r=smaug" for causing wpt failures @ history_pushstate_too_many_calls.optional.html

Comment 11

[amarc]

Backed out for causing wpt failures

• Backout link
• Push with failures
• Failure Log
• Failure Log 2

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f570a96ebc39
https://hg.mozilla.org/integration/autoland/rev/fc619db3ad61
Increase navigation rate limiting threshold. r=smaug

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/60eb935df843
https://hg.mozilla.org/integration/autoland/rev/43e9ea3e063e
Revert "Bug 1995636 - Increase navigation rate limiting threshold. r=smaug" for causing failures at history_replacestate_too_many_calls.optional.html.

Comment 14

[Atila Butkovits]

Backed out for causing failures at history_replacestate_too_many_calls.optional.html.

Backout link: https://hg.mozilla.org/integration/autoland/rev/43e9ea3e063e

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=fc619db3ad614c92f8242e0137023491ff8761e3&selectedTaskRun=JyYvq2yUSWyzntvVjiNotg.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=545911146&repo=autoland&task=JyYvq2yUSWyzntvVjiNotg.0&lineNumber=1823

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ab959ec6edc9
https://hg.mozilla.org/integration/autoland/rev/6c562728e10c
Increase navigation rate limiting threshold. r=smaug,zcorpan

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/57415 for changes under testing/web-platform/tests

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f2fcc4ff3fbc
https://hg.mozilla.org/integration/autoland/rev/69ee01432c73
Revert "Bug 1995636 - Increase navigation rate limiting threshold. r=smaug,zcorpan" for causing wpt failures on replaceState-inside-back-handler-infinite.optional.html

Comment 18

[rperta]

Backed out for causing wpt failures on replaceState-inside-back-handler-infinite.optional.html
Backout link
Push with failures
Failure log(s)

Comment 19

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e13e3a6b642f
https://hg.mozilla.org/integration/autoland/rev/20c3997c6508
Increase navigation rate limiting threshold. r=smaug,zcorpan

Comment 21

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/57439 for changes under testing/web-platform/tests

Comment 22

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/20c3997c6508

Comment 23

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 24

[Donal Meehan [:dmeehan]]

:farre, could you consider adding a beta uplift request on this if it's low risk?
It seems to fix a few web compact issues

Comment 25

[Andreas Farre [:farre]|PTO -> Aug 7th]

Attachment: Bug 1995636 - Increase navigation rate limiting threshold.

Original Revision: https://phabricator.services.mozilla.com/D280599

Comment 26

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: Less web compatibility
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: It's a preference change, allowing for more calls to pushState and replaceState during a set period of time.
• String changes made/needed: None
• Is Android affected?: yes

Comment 27

[Andreas Farre [:farre]|PTO -> Aug 7th]

Attachment: Bug 1995636 - Increase navigation rate limiting threshold.

Original Revision: https://phabricator.services.mozilla.com/D280599

Comment 28

[Andreas Farre [:farre]|PTO -> Aug 7th]

Thanks for the remainder.

I went ahead and requested release, and the esr:s as well. It's small enough to warrant inclusion on all of them. Sounds good?

Comment 29

[Phabricator Automation]

firefox-release Uplift Approval Request

• User impact if declined: Less web compatibility
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: It's a preference change, allowing for more calls to pushState and replaceState during a set period of time.
• String changes made/needed: None
• Is Android affected?: yes

Comment 30

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/9bb681af6441
https://hg.mozilla.org/releases/mozilla-beta/rev/b22d89e68aeb

Comment 31

[Donal Meehan [:dmeehan]]

:farre, it looks like you wanted to created uplift requests for ESR115/ESR140, but they failed due to conflicts.
See details here https://lando.moz.tools/D281585/

Comment 32

[Ryan VanderMeulen [:RyanVM]]

I also don't think this is an appropriate item to backport to ESR115 as it reaches EOL (assuming it's even affected in the first place?)

Comment 33

[Andreas Farre [:farre]|PTO -> Aug 7th]

Attachment: Bug 1995636 - Increase navigation rate limiting threshold.

Original Revision: https://phabricator.services.mozilla.com/D280599

Comment 34

[Phabricator Automation]

firefox-esr140 Uplift Approval Request

• User impact if declined: Less web compatibility
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: It's a preference change, allowing for more calls to pushState and replaceState during a set period of time.
• String changes made/needed: None
• Is Android affected?: yes

Comment 35

[Andreas Farre [:farre]|PTO -> Aug 7th]

(In reply to Donal Meehan [:dmeehan] from comment #31)

:farre, it looks like you wanted to created uplift requests for ESR115/ESR140, but they failed due to conflicts.
See details here https://lando.moz.tools/D281585/

Thanks, fixed and followed resolution steps. As Ryan says, we should not consider this for 115.

Comment 36

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/b7fbdaf1aadd

Comment 37

[Andreas Farre [:farre]|PTO -> Aug 7th]

Attachment: Bug 1995636 - Increase navigation rate limiting threshold.

Original Revision: https://phabricator.services.mozilla.com/D280599

Comment 38

[Phabricator Automation]

firefox-esr115 Uplift Approval Request

• User impact if declined/Reason for urgency: Less web compatibility
• Code covered by automated testing?: yes
• Fix verified in Nightly?: yes
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: It's a preference change, allowing for more calls to pushState and replaceState during a set period of time.
• String changes made/needed?: None
• Is Android affected?: yes

Comment 39

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/27e16a7daff6