openUILinkIn code in netmonitor looks unsued

RESOLVED FIXED in Firefox 63

Status

P2
normal
RESOLVED FIXED
7 months ago
7 months ago

People

(Reporter: jkt, Assigned: jkt)

Tracking

(Blocks: 1 bug)

63 Branch
Firefox 63

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 months ago
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.
(Assignee)

Comment 1

7 months ago
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
(Assignee)

Comment 5

7 months ago
> 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)

Updated

7 months ago
Assignee: nobody → jkt
(Assignee)

Updated

7 months ago
Blocks: 1333030
(Assignee)

Updated

7 months ago
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+

Comment 7

7 months ago
Pushed by jkingston@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/80282ed48585
fix devtools to use openWebLinkIn. r=Honza

Comment 8

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/80282ed48585
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.