Pretty print context menu item shouldn't display for non-JS files
Categories
(DevTools :: Debugger, enhancement, P3)
Tracking
(Not tracked)
People
(Reporter: davidwalsh, Assigned: davidwalsh)
Details
Attachments
(1 file, 1 obsolete file)
We re-implemented the shim but naively -- we shouldn't show the pretty print context menu item for tabs if the source isn't JS.
|
Assignee | |
Comment 1•5 years ago
|
||
|
|
Assignee | |
Updated•5 years ago
|
Pushed by dwalsh@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/33f142760b69 Don't show pretty print option for non-JS sources r=jlast
|
||
Comment 3•5 years ago
|
||
David, will this handle webpack:///webpack/bootstrap or webpack:///public/locales%20lazy%20^\.\/.*\/send\.ftl$%20namespace%20object in https://send.firefox.com/ , which are JS file but do not have the extension. I don't have examples of minified files; but build steps are easy to mess up.
The main concern is that the context menu is the last resort for Debugger's pretty print detection missing prettifiable files; and this patch depends on JS being properly detected – which it doesn't seem for the files above.
|
||
Comment 4•5 years ago
|
||
| bugherder | ||
|
|
Assignee | |
Comment 5•5 years ago
|
||
:digitarald You're right -- it doesn't do those URLs. My understanding is we wanted this shim for cases where sourcemaps failed. Would you like a revert of this?
|
|
||
Comment 6•5 years ago
|
||
My understanding is we wanted this shim for cases where sourcemaps failed. Would you like a revert of this?
How far do you trust that source maps are always pointing to the pretty printed results?
Maybe I am missing a reason that this should really be hidden. The main risk here is that this is a last-resort power-user option from a secondary interface; which would cause any harm if it doesn't work on some files.
|
|
Assignee | |
Comment 7•5 years ago
|
||
I hid this after testing https://davidwalsh.name :
- We show an
(index) - I right-click the tab, pretty print, and then see "Loading....." forever
... because we don't pretty-print HTML. Or XML.
|
|
Assignee | |
Comment 8•5 years ago
|
||
I talked to Harald and he doesn't wan this.
|
||
Comment 9•5 years ago
|
||
Chrome auto-detects JS in the bootstrap case, which might be interesting investigating.
|
|
||
Comment 10•5 years ago
|
||
I talked to Harald and he doesn't wan this.
To add some context for posterity: Anything that could potentially impact productivity for the web compat team needs to be considered carefully. Showing/hiding the pretty-print button correctly is a long-standing issue, making the context menu a last-resort option to force pretty printing.
|
||
Comment 11•5 years ago
|
||
Pushed by dwalsh@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cbcf5b79aa44 Backed out changeset 33f142760b69 - Don't check for JavaScript before showing debugger source tab title r=jlast
|
||
Comment 12•5 years ago
|
||
| bugherder | ||
|
||
Updated•5 years ago
|
|
|
||
Comment 13•5 years ago
|
||
|
|
||
Comment 14•5 years ago
|
||
ni? David, given that bug 1539179 appeared in DevEdition; could it be that the patch made it into 67 but the backout didn't?
|
|
Assignee | |
Comment 15•5 years ago
|
||
Removing that JS check is in our uplift list: https://bugzilla.mozilla.org/show_bug.cgi?id=1539813
|
|
||
Comment 16•5 years ago
|
||
I believe this is fixed.
|
||
Updated•5 years ago
|
Description
•