Closed
Bug 1486159
Opened 6 years ago
Closed 6 years ago
openUILinkIn code in netmonitor looks unsued
Categories
(DevTools :: Netmonitor, enhancement, P2)
Tracking
(firefox63 fixed)
RESOLVED
FIXED
Firefox 63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: jkt, Assigned: jkt)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
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•6 years 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)
Comment 2•6 years ago
|
||
(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
Comment 3•6 years ago
|
||
It looks like we should use:
top.openWebLinkIn(link, "tab");
...instead of `openUILinkIn`
Honza
Assignee | ||
Comment 4•6 years ago
|
||
Assignee | ||
Comment 5•6 years 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•6 years ago
|
Assignee: nobody → jkt
Assignee | ||
Updated•6 years ago
|
Blocks: require-triggering-principal
Assignee | ||
Updated•6 years ago
|
Status: NEW → ASSIGNED
Priority: P3 → P2
Comment 6•6 years ago
|
||
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
Comment 8•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years 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.
Description
•