Don't special case XUL documents in webExtensionTargetPrototype._shouldAddNewGlobalAsDebuggee

RESOLVED FIXED in Firefox 68

Status

task
RESOLVED FIXED
2 months ago
2 months ago

People

(Reporter: bgrins, Assigned: bgrins)

Tracking

unspecified
Firefox 68
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

2 months ago

See https://bugzilla.mozilla.org/show_bug.cgi?id=1542461#c15:

Luca, in webExtensionTargetPrototype._shouldAddNewGlobalAsDebuggee we have:
ChromeUtils.getClassName(global.document) == "XULDocument"
But as we convert more chrome documents to HTML (i.e. browser.xhtml instead of browser.xul) this check won't work anymore. AFAICT this function will still end up returning false in this case due to this.isExtensionWindowDescendent(global.document.ownerGlobal) later on for these documents, but I'm wondering if we should either remove the "XULDocument" check or change it to check for system principal. I'm not sure if there are gotchas with doing the latter (like, extension windows that are also system principal). Do you have an opinion on what to do here?

And https://bugzilla.mozilla.org/show_bug.cgi?id=1542461#c16:

I've been trying to remember if there were any other reasons for that check, but from what I recall it was meant to just exit earlier (without further checks) on a global related to a XUL document.
As you pointed out there are also the two checks that follows it which are still going to filter out any of those documents, and so it seems to me that we could just remove the explicit check for the XUL documents.

Assignee

Updated

2 months ago
Type: defect → task
Assignee

Comment 1

2 months ago

As we convert more chrome documents away from XUL we end up running
through two different paths in this function. These are going to be
filtered out in later checks anyway, so this change removes the early return.

Comment 2

2 months ago
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/24125453aa81
Don't special case XUL documents in webExtensionTargetPrototype._shouldAddNewGlobalAsDebuggee;r=rpl

Comment 3

2 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
You need to log in before you can comment on or make changes to this bug.