Closed
Bug 1134619
Opened 9 years ago
Closed 9 years ago
WebIDE can't debug Chrome when settings are open
Categories
(DevTools Graveyard :: WebIDE, defect)
Tracking
(firefox39 fixed)
RESOLVED
FIXED
Firefox 39
Tracking | Status | |
---|---|---|
firefox39 | --- | fixed |
People
(Reporter: past, Assigned: past)
Details
Attachments
(1 file)
1.47 KB,
patch
|
ochameau
:
review+
|
Details | Diff | Splinter Review |
Opening WebIDE and connecting to Chrome with the Settings page open in a tab I get this error and the tab list fails to appear: JPM error JavaScript error: chrome://webide/content/webide.js, line 1219: TypeError: chrome://settings/ is not a valid URL. That line contains: 1219: let url = new URL(tab.url); The debugger is much less strict in handling tab URLs: https://dxr.mozilla.org/mozilla-central/source/browser/devtools/debugger/debugger-panes.js#1793 Can't we do something similar here? Note that Chrome devtools properly open and debug such pages, just like we can debug about: pages. At the very least, we should try/catch around that line and continue with the rest of the open tabs.
Hmm, it seems the platform explicitly blocks new URL() from parsing chrome:// URLs. :/ Any random scheme name works, so chrome must be banned for some reason. Anyway, we should make this work one way or another.
Assignee | ||
Comment 2•9 years ago
|
||
It turns out that chrome: URLs are parsed properly, but only those that are actually valid for Firefox (which chrome://settings is not). Therefore I just ignore these URLs, since I can't think of a way to debug them properly. I couldn't write a test for the same reason: using something like chrome://webide/content would work even without the patch and using chrome://settings would throw when trying to create the tab in Firefox.
Attachment #8572635 -
Flags: review?(poirot.alex)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → past
Status: NEW → ASSIGNED
Comment 3•9 years ago
|
||
Comment on attachment 8572635 [details] [diff] [review] Don't let WebIDE break when trying to debug Chrome settings Review of attachment 8572635 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me, especially if these pages can't be debugged. Otherwise it would be better to fix that code to be able to display a meaningful label instead of skipping them. Also, may be valence should only expose the tabs it is able to debug?
Attachment #8572635 -
Flags: review?(poirot.alex) → review+
Assignee | ||
Comment 4•9 years ago
|
||
Both good points. It turns out that these pages can indeed be debugged, it's just the tab list that can't handle them. I have a fix in Valence for this and I'll land this patch as a safeguard that WebIDE never breaks in the presence of an invalid URL.
Assignee | ||
Comment 5•9 years ago
|
||
Here is the change I mention above: https://github.com/campd/valence/pull/167 Can you review that too, please?
Assignee | ||
Comment 6•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/e278a83c3975
Comment 7•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e278a83c3975
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
Updated•6 years ago
|
Product: Firefox → DevTools
Updated•4 years ago
|
Product: DevTools → DevTools Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•