Closed
Bug 1488974
Opened 6 years ago
Closed 6 years ago
Disable FastBlock after the load event is triggered
Categories
(Core :: Networking, defect, P1)
Core
Networking
Tracking
()
RESOLVED
FIXED
mozilla64
People
(Reporter: francois, Assigned: francois)
References
Details
(Whiteboard: [necko-triaged])
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
ehsan.akhgari
:
review+
mayhemer
:
review+
pascalc
:
approval-mozilla-beta+
|
Details |
We should disable FastBlock as soon as the load event for the page has fired.
This will let some trackers load (the one which are delayed until after the page has finished loading).
Assignee | ||
Comment 1•6 years ago
|
||
My plan is to copy baku's patch from bug 1485492 (https://hg.mozilla.org/mozilla-central/rev/3b26f3da46d8) and add a hasDocumentLoaded field to the LoadInfo object.
I would set that field to true in LoadInfo from within nsIDocument::UnblockDOMContentLoaded().
Ehsan, does that sound good to you or is there an easier way to do this?
Flags: needinfo?(ehsan)
Comment 2•6 years ago
|
||
That sounds about right. One question though, did you want to capture the load event or DOMContentLoaded? The above is for the former. The latter can be caught here: https://searchfox.org/mozilla-central/rev/de7676288a78b70d2b9927c79493adbf294faad5/dom/base/nsGlobalWindowInner.cpp#2076
Flags: needinfo?(ehsan)
Comment 3•6 years ago
|
||
This would probably break the test cases in bug 1477046, which adjusts browser.fastblock.timeout dynamically.
Assignee | ||
Comment 4•6 years ago
|
||
The test used to assume that the load event didn't matter and so
the expected values had to be updated to match the new behavior.
A new "slowIFrame" test was added to capture what was previously
tested by the "badIFrame".
Assignee | ||
Comment 5•6 years ago
|
||
Manually tested using https://mozilla.github.io/tracking-test/fastblock-bug1488974.html
Comment 6•6 years ago
|
||
Comment on attachment 9012435 [details]
Bug 1488974 - Disable FastBlock after the load event has fired. r?ehsan,mayhemer
:Ehsan Akhgari has approved the revision.
Attachment #9012435 -
Flags: review+
Comment 7•6 years ago
|
||
Comment on attachment 9012435 [details]
Bug 1488974 - Disable FastBlock after the load event has fired. r?ehsan,mayhemer
Honza Bambas (:mayhemer) has approved the revision.
Attachment #9012435 -
Flags: review+
Pushed by fmarier@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0c1057bba224
Disable FastBlock after the load event has fired. r=mayhemer,Ehsan
Comment 9•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Assignee | ||
Comment 10•6 years ago
|
||
Comment on attachment 9012435 [details]
Bug 1488974 - Disable FastBlock after the load event has fired. r?ehsan,mayhemer
Approval Request Comment
[Feature/Bug causing the regression]: FastBlock
[User impact if declined]: Scripts, iframes, XHRs that occur after a page load can get blocked by FastBlock leading to various page breakage.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes, see comment 5.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Not very.
[Why is the change risky/not risky?]: It's a relatively straightforward change and is covered by mochitests.
[String changes made/needed]: None.
Attachment #9012435 -
Flags: approval-mozilla-beta?
Comment 11•6 years ago
|
||
Comment on attachment 9012435 [details]
Bug 1488974 - Disable FastBlock after the load event has fired. r?ehsan,mayhemer
P1 on a feature for which we conduct a shield study on 63, uplift accepted for 63 beta 11, thanks.
Attachment #9012435 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 12•6 years ago
|
||
bugherder uplift |
status-firefox63:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•