Closed Bug 1430117 Opened 2 years ago Closed 2 years ago

Checking NS_BINDING_ABORTED to avoid dispatching navigate event

Categories

(DevTools :: Framework, enhancement, P3)

enhancement

Tracking

(firefox59 fixed)

RESOLVED FIXED
Firefox 59
Tracking Status
firefox59 --- fixed

People

(Reporter: rickychien, Assigned: rickychien)

References

Details

Attachments

(1 file)

Follow-up for fixing the suggested solution from https://bugzilla.mozilla.org/show_bug.cgi?id=1409705#c10.
If you'd like to try out the patch, please rebase on top of bug 1409705.
Comment on attachment 8942183 [details]
Bug 1430117 - Checking NS_BINDING_ABORTED to avoid dispatching navigate event

https://reviewboard.mozilla.org/r/212464/#review218534

Thanks for the patch, it would have been great to have a test, but it seems to complex to catch the actor error and make the test fail.

::: devtools/server/actors/tab.js:1639
(Diff revision 4)
>        this._tabActor._willNavigate(window, newURI, request);
>      }
>      if (isWindow && isStop) {
>        // Don't dispatch "navigate" event just yet when there is a redirect to
>        // about:neterror page.
> -      if (request.status != Cr.NS_OK) {
> +      if (request.status != Cr.NS_OK && request.status != Cr.NS_BINDING_ABORTED) {

Could you add a comment explaining the NS_BINDING_ABORTED, something like:
 // Navigating to about:neterror will make `status` be something else than NS_OK. But for some error like NS_BINDING_ABORTED we don't want to emit any `navigate` event as the page load has been cancelled and the related page document is going to be a dead wrapper.
Attachment #8942183 - Flags: review?(poirot.alex) → review+
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/27e396761ed3
Checking NS_BINDING_ABORTED to avoid dispatching navigate event r=ochameau
https://hg.mozilla.org/mozilla-central/rev/27e396761ed3
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.