Closed Bug 1569775 Opened 5 years ago Closed 5 months ago

Make "unload" event listener breakpoint work

Categories

(DevTools :: Debugger, enhancement, P3)

enhancement

Tracking

(firefox122 fixed)

RESOLVED FIXED
122 Branch
Tracking Status
firefox122 --- fixed

People

(Reporter: miketaylr, Assigned: ochameau)

References

(Blocks 1 open bug)

Details

(Whiteboard: [debugger-reserve])

Attachments

(2 files, 1 obsolete file)

STR:

  1. go to google.com (any page is fine)
  2. set "unload" breakpoint from Event Listener Breakpoints panel
  3. hit refresh

Expected:
Debugger hits the breakpoint and I can do things

Actual:
Debugger hits the breakpoint, but the Debugger panel is blank.

I dunno if the panel is trying to shut itself down or what, but it would be useful for Bug 1562432 to be able to debug unload events.

We are probably racing other unload logic.

When I try to pause on onbeforeunload and unload I end up paused but with an blank debugger tab and Browser Console errors:

Error: Page has navigated context.js:35:49
Error: "source sourceURL-https://www.google.com/ does not exist"
    getSourceFromId resource://devtools/client/debugger/src/reducers/sources.js:451
    mapStateToProps resource://devtools/client/debugger/src/components/SecondaryPanes/index.js:465
    Redux 10
    waitUntilService resource://devtools/client/debugger/src/actions/utils/middleware/wait-service.js:65
    promiseMiddleware resource://devtools/client/debugger/src/actions/utils/middleware/promise.js:31
    context resource://devtools/client/debugger/src/actions/utils/middleware/context.js:33
    thunk resource://devtools/client/debugger/src/actions/utils/middleware/thunk.js:21
    dispatch Redux
    paused resource://devtools/client/debugger/src/actions/pause/paused.js:32
    thunk resource://devtools/client/debugger/src/actions/utils/middleware/thunk.js:21
    bindActionCreator Redux
    paused resource://devtools/client/debugger/src/client/firefox/events.js:64
react-dom.js:12769:13
Error: source sourceURL-https://www.google.com/ does not exist sources.js:451:11
Priority: -- → P2

Testing with https://firefox-devtools-test-unload-event.glitch.me/ in Chrome

  1. When refreshing neither beforeunload or unload can be paused on
  2. When navigating (by clicking the Glitch link only beforeunload is being paused on.
  3. When closing the tab or the window, both events are being paused correctly.

In Firefox only beforeunload was handled correctly, unload always causes a blank debugger panel.

Priority: P2 → P1
Blocks: dbg-70
Whiteboard: [debugger-reserve]

@digitarald
A proper fix for this will probably take a good amount of time and exploration. What would you think about uplifting a patch removing these two events from the list of available event BPs, so that users don't run into this?

With that, we can set this back to a P2/3 and then treat the new uplift/removal as a P1.

Flags: needinfo?(hkirschner)

P3 is good for now, unload as this isn't a regression but a complex corner case in a new feature.

Flags: needinfo?(hkirschner)

@digitarald

To clarify, the core issue right now is, it's not just a case that we don't handle, it can actually crash the debugger:

Actual:
Debugger hits the breakpoint, but the Debugger panel is blank.

I agree that making it work correctly is a P3, but I'm saying we should have a P1 uplift to remove that checkbox until it actually works, since right how it's just a checkbox that may crash the devtools when you use it and have an event handler of that type.

Flags: needinfo?(hkirschner)

Removing these problematic events until they work seems like a good idea.

Attached image image.png

Only concern is that this works with iframes atm (tested with glitch and their inline preview in https://glitch.com/~firefox-devtools-testcase-unload and clicking the Glitch link).

Alternative is to make the Debugger handle this case more gracefully, ignoring unload pausing and not ending in this bad paused state. This is backed up by another yet similar issue where the same breakage can be caused by a debugger statement in an unload event: https://firefox-devtools-testcase-unload.glitch.me/

Flags: needinfo?(hkirschner)
Assignee: nobody → lsmyth
Status: NEW → ASSIGNED
Depends on: 1575096
Assignee: lsmyth → nobody
Blocks: dbg-71
No longer blocks: dbg-70
Status: ASSIGNED → NEW
Priority: P1 → P3

Comment on attachment 9086543 [details]
Bug 1569775 - Disable beforeunload/unload event-breakpoint UI until fixed. r=jlast

Revision D42577 was moved to bug 1575096. Setting attachment 9086543 [details] to obsolete.

Attachment #9086543 - Attachment is obsolete: true
Whiteboard: [debugger-reserve]
Whiteboard: [debugger-mvp]
Whiteboard: [debugger-mvp] → [debugger-reserve]
No longer blocks: dbg-71
Type: defect → enhancement
Summary: Can't set "unload" event listener breakpoint (and do useful things once it's hit) → Make "unload" event listener breakpoint work
Severity: normal → S3
See Also: → 1806796

Support for breakpoint while unloading the page has been fixed by bug 1806796.
This is only matter of enabling these event types in the UI and covering them by tests.

Assignee: nobody → poirot.alex
Status: NEW → ASSIGNED

Note that bug 1806796 and dependencies had fixed support for setting manual breakpoints or debugger statements in unload events for 1/2monthes.
This patch is solely to enable the unload/beforeunload items in the event breakpoint list.

Depends on: 1861941
Depends on: 1861943

This, or something similar is pretty critical for debugging popup windows.

I needed to be able to see the network events during an oauth 2 flow in a popup window. But the window closed before I had a chance to inspect the network tab. I couldn't find any way to pause the window before it automatically closed.

I eventually had to switch to chrome and add a breakpoint on window.close in the Event Listener breakpoints to get something useful. But as far as I can tell Firefox doesn't have anything equivalent.

(In reply to Thayne from comment #13)

I eventually had to switch to chrome and add a breakpoint on window.close in the Event Listener breakpoints to get something useful. But as far as I can tell Firefox doesn't have anything equivalent.

Did you try on Firefox 120 or more?
Improvements made to unload breakpoints only landed recently.
We don't support the "unload" breakpoints in the event listener list yet (this should be fixed soon in this bug, hopefully Fx121).
In the meantime you can already set a manual breakpoint on code that is triggerred on unload. If you don't own the page, you can evaluate this in this console window.onunload = ()=> debugger;.

Otherwise, regarding popup windows, there is an experimental features which helps debugging popups.
You may give it a try by toggling devtools.popups.debug preference, but I see you were already aware of it via bug 1569859.

Did you try on Firefox 120 or more?

No. 119

If you don't own the page, you can evaluate this in this console window.onunload = ()=> debugger;

unfortunately, that won't work for me. Because the window is closed immediately after a redirect to a page I don't control. So I don't have a chance to set any breakpoints before it closes.

Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3df03c7436cf
[devtools] Enable beforeunload and unload event breakpoints. r=devtools-reviewers,nchevobbe
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 122 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: