874028 - Debugger tests use load event on tab (and therefore favicon) rather than on tab's browser to detect pageload

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[:Gijs (he/him)]

The debugger's head.js addTab code contains this:

72   let tab = targetBrowser.selectedTab;
73   let win = tab.linkedBrowser.contentWindow;
74   let expectedReadyState = aURL == "about:blank" ? ["interactive", "complete"] : ["complete"];
75 
76   if (aOnload) {
77     let handler = function() {
78       if (tab.linkedBrowser.currentURI.spec != aURL ||
79           expectedReadyState.indexOf((win.document || {}).readyState) == -1) {
80         return;
81       }
82       tab.removeEventListener("load", handler, false);
83       executeSoon(aOnload);
84     }
85     tab.addEventListener("load", handler, false);
86   }

Unfortunately, the tab (which is just the tabbrowser's <tab> element) doesn't actually have load events, except for the tab's favicon. The fact that this currently works at all is probably mostly due to the expectedReadyState checking and a miracle. Because of how we remove empty favicons on the UX branch, it completely fails to work there. It should probably listen to the tab's linkedBrowser's load event instead.

Comment 1

[:Gijs (he/him)]

Attachment: Patch

This fixes the UX branch, AFAICT. I've also updated the netmonitor test; I don't believe any other tests are using the same code.

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 751672 [details] [diff] [review]
Patch

Review of attachment 751672 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 3

[:Gijs (he/him)]

Pushed: https://hg.mozilla.org/integration/fx-team/rev/108c6b3bf66e

Thanks for the quick r+!

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/108c6b3bf66e