openUILinkIn code in netmonitor looks unsued

RESOLVED FIXED in Firefox 63

Status

enhancement
P2
normal
RESOLVED FIXED
11 months ago
11 months ago

People

(Reporter: jkt, Assigned: jkt)

Tracking

(Blocks 1 bug)

63 Branch
Firefox 63
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment)

openUILinkIn code looks unused https://searchfox.org/mozilla-central/rev/1410bb760a5e77236b74999807f5500bd285a57d/devtools/client/netmonitor/src/app.js#35-39

"toolbox-panel-iframe-netmonitor" looks like the element no longer exists, unless this is added by an extension or dynamically?

openUILinkIn with two params also now throws an exception.
This looks to me like safe code we can remove? I see the function itself the code is generating appears to be passed around a lot. Should all of this be removed?
Flags: needinfo?(odvarko)
(In reply to Jonathan Kingston [:jkt] from comment #0)
> openUILinkIn code looks unused
> https://searchfox.org/mozilla-central/rev/
> 1410bb760a5e77236b74999807f5500bd285a57d/devtools/client/netmonitor/src/app.
> js#35-39
It's still being used. This allows the user to open links stored e.g. in HTTP headers, Cookies, HTTP responses, etc. by clicking on them. 

You can e.g. try "Referer" HTTP header.

Here is the function that eventually calls the 'openLink' callback.

https://github.com/devtools-html/debugger.html/blob/e7f212ee3916d97c799549b6b89cb43fbc1ce34f/packages/devtools-reps/src/reps/string.js#L209

> "toolbox-panel-iframe-netmonitor" looks like the element no longer exists,
> unless this is added by an extension or dynamically?
This element represents the iframe the Net monitor UI lives in.
(created dynamically)

> openUILinkIn with two params also now throws an exception.

Yes, this sounds like a bug.

I am seeing:
Error: Required argument triggeringPrincipal missing within openUILinkIn

Is this the exception you are seeing?

Honza
Flags: needinfo?(odvarko) → needinfo?(jkt)
Priority: -- → P3
It looks like we should use:

top.openWebLinkIn(link, "tab");

...instead of `openUILinkIn`

Honza
> It looks like we should use:

Yup just couldn't figure out if the code was being used.

We should probably do a follow up bug to write a test for this functionality so it doesn't happen again.
Flags: needinfo?(jkt)
Assignee: nobody → jkt
Status: NEW → ASSIGNED
Priority: P3 → P2
Comment on attachment 9004385 [details]
Bug 1486159 - fix devtools to use openWebLinkIn. r?honza

Jan Honza Odvarko [:Honza] has approved the revision.
Attachment #9004385 - Flags: review+
Pushed by jkingston@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/80282ed48585
fix devtools to use openWebLinkIn. r=Honza
https://hg.mozilla.org/mozilla-central/rev/80282ed48585
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.