1670061 - Beforeunload prompt isTabModalPromptAllowed check broken

Status: RESOLVED INVALID

(Core :: DOM: Navigation, defect)

Description

[Paul Zühlcke [:pbz]]

In PromptCollection.asyncBeforeUnloadCheck, the check for contentViewer.isTabModalPromptAllowed does not work, since contentViewer is always null.

https://searchfox.org/mozilla-central/rev/919607a3610222099fbfb0113c98b77888ebcbfb/browser/components/prompts/PromptCollection.jsm#96,101

I'm assuming that's because the docshell associated with the browsing context is not accessible from the parent process?

Comment 1

[Kris Maglione [:kmag]]

That shouldn't be a problem. It's expected in the out-of-process case, which is why we check the BrowsingContext instead when there isn't a content viewer.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1655866

Comment 3

[Paul Zühlcke [:pbz]]

Doesn't this flag have a different meaning? Looking at the getter it returns the visibility state of the content viewer: https://searchfox.org/mozilla-central/rev/d25eb00ab4e90cc0130cd18f303a04cc2a2f8409/layout/base/nsDocumentViewer.cpp#3756

Could you please elaborate on why we don't need to check it in the out-of-process case? I assume that is the common case.
Thanks!

Comment 4

[Kris Maglione [:kmag]]

(In reply to Paul Zühlcke [:pbz] from comment #3)

Doesn't this flag have a different meaning? Looking at the getter it returns the visibility state of the content viewer: https://searchfox.org/mozilla-central/rev/d25eb00ab4e90cc0130cd18f303a04cc2a2f8409/layout/base/nsDocumentViewer.cpp#3756

No, the two checks have the same effect.

Comment 5

[Pascal Chevrel:pascalc]

Kris, do we care about this regression for 83? Thanks

Comment 6

[Paul Zühlcke [:pbz]]

(In reply to Kris Maglione [:kmag] from comment #4)

(In reply to Paul Zühlcke [:pbz] from comment #3)

Doesn't this flag have a different meaning? Looking at the getter it returns the visibility state of the content viewer: https://searchfox.org/mozilla-central/rev/d25eb00ab4e90cc0130cd18f303a04cc2a2f8409/layout/base/nsDocumentViewer.cpp#3756

No, the two checks have the same effect.

Thanks! In this case the bug is invalid. However, we should discuss whether it makes sense to keep both flags or to skip prompting in these situations altogether in Bug 1666374.