Closed
Bug 1258154
Opened 9 years ago
Closed 9 years ago
Clicking function in console when debugger is on opens empty tab instead of object inspector
Categories
(DevTools :: Console, defect, P2)
Tracking
(firefox48 fixed)
RESOLVED
FIXED
Firefox 48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: Oriol, Assigned: bgrins)
References
Details
(Keywords: regression, Whiteboard: [btpp-fix-later])
Attachments
(2 files)
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:48.0) Gecko/20100101 Firefox/48.0
Build ID: 20160319030558
Steps to reproduce:
1. Open Web Console
2. Enter (function(){})
3. Click the returned object
Actual results:
New empty tab is opened.
Expected results:
Object inspect should have inspected the function object.
Browser Console shows this error:
NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIWebNavigation.loadURI]
Pushlog: https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=89c1cc2faea8903570688dd38d9f71ec0c5f0b87&tochange=256e3e81fed77b17ea9158786a349698bb59a6ab
Reporter | ||
Updated•9 years ago
|
Assignee | ||
Comment 1•9 years ago
|
||
Thanks for filing. I can confirm the issue in a Nightly build. It looks like it's trying to load the object up in the Debugger instead of the Variables View since it has a location (https://dxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/console-output.js#2781), which results in the new empty tab.
My guess is we need to treat objects who's location was created in a debugger eval frame as if they don't have a location. James, does that seem right? If so, how can we check for that?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(jlong)
Assignee | ||
Comment 2•9 years ago
|
||
AFAICT this is only affecting functions that are returned from an evaluation in the console, but it's a pretty ugly failure
Priority: -- → P2
Whiteboard: [btpp-fix-later]
Reporter | ||
Comment 3•9 years ago
|
||
Bug 1132501 made this easier to reproduce, but adding an additional step the real culprit seems bug 1050691
1. Open Debugger
2. Open Web Console
3. Enter (function(){})
4. Click the returned object
Blocks: 1050691
Assignee | ||
Comment 4•9 years ago
|
||
Yeah, Bug 1132501 seems like the right issue
No longer blocks: 1050691
Reporter | ||
Comment 5•9 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #4)
> Yeah, Bug 1132501 seems like the right issue
I think you meant bug 1050691
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(fayolle-florent)
Assignee | ||
Comment 6•9 years ago
|
||
(In reply to Oriol from comment #5)
> (In reply to Brian Grinstead [:bgrins] from comment #4)
> > Yeah, Bug 1132501 seems like the right issue
>
> I think you meant bug 1050691
Oops, thanks. Clearing the ni? for Florent for now though - let's see what James says about how to fix it.
Flags: needinfo?(fayolle-florent)
Comment 7•9 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #1)
> Thanks for filing. I can confirm the issue in a Nightly build. It looks
> like it's trying to load the object up in the Debugger instead of the
> Variables View since it has a location
> (https://dxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/
> console-output.js#2781), which results in the new empty tab.
>
> My guess is we need to treat objects who's location was created in a
> debugger eval frame as if they don't have a location. James, does that seem
> right? If so, how can we check for that?
Even easier: I think we can just check if `location.source.url` is null. If it's null we won't be able to display it in the debugger. Well, we actually could make that work but it would be a lot more work. The debugger should show eval scripts correctly but that path isn't working here.
I'd say just do the quick fix for now: open the variables view if the URL is null.
Flags: needinfo?(jlong)
Assignee | ||
Comment 8•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/41561/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/41561/
Attachment #8733115 -
Flags: review?(jlong)
Assignee | ||
Comment 9•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/41563/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/41563/
Attachment #8733116 -
Flags: review?(jlong)
Assignee | ||
Comment 10•9 years ago
|
||
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Comment 11•9 years ago
|
||
Comment on attachment 8733116 [details]
MozReview Request: Bug 1258154 - Minor test cleanups for function inspection in console;r=jlongster
https://reviewboard.mozilla.org/r/41563/#review38209
Attachment #8733116 -
Flags: review?(jlong) → review+
Comment 12•9 years ago
|
||
Comment on attachment 8733115 [details]
MozReview Request: Bug 1258154 - Open functions in variables view if they were returned from console evaluation;r=jlongster
https://reviewboard.mozilla.org/r/41561/#review38211
::: devtools/client/webconsole/console-output.js:2782
(Diff revision 1)
> this._text(")");
> },
>
> _onClick: function () {
> let location = this.objectActor.location;
> - if (location) {
> + if (location && location.source && location.source.url) {
When is `location.source` null? If there's a location it should definitely have a source.
Attachment #8733115 -
Flags: review?(jlong) → review+
Assignee | ||
Comment 13•9 years ago
|
||
(In reply to James Long (:jlongster) from comment #12)
> Comment on attachment 8733115 [details]
> MozReview Request: Bug 1258154 - Open functions in variables view if they
> were returned from console evaluation;r=jlongster
>
> https://reviewboard.mozilla.org/r/41561/#review38211
>
> ::: devtools/client/webconsole/console-output.js:2782
> (Diff revision 1)
> > this._text(")");
> > },
> >
> > _onClick: function () {
> > let location = this.objectActor.location;
> > - if (location) {
> > + if (location && location.source && location.source.url) {
>
> When is `location.source` null? If there's a location it should definitely
> have a source.
I bumped into this case when following STR in the bug, so entering `(function(){})` in the console
Comment 14•9 years ago
|
||
`location.source.url` should be null. It should have `location.source`, does it not? I'm curious because we should doing this consistently by now. There is a source there, but it just doesn't have a URL. If not I might look into why there is no source (but it's fine to land this with all the checks).
Assignee | ||
Comment 15•9 years ago
|
||
Weird, devtools/client/webconsole/test/browser_webconsole_bug_1050691_click_function_to_source.js failed with this applied and it looks like in that case we need to check for 'location.url' and not 'location.source.url': https://treeherder.mozilla.org/#/jobs?repo=try&revision=74311c0248c2&selectedJob=18379636
Assignee | ||
Comment 16•9 years ago
|
||
Trying to use location.url and IGNORED_SOURCE_URLS here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=795aa35c997d / https://hg.mozilla.org/try/rev/f18c4558156f#l1.13
Assignee | ||
Comment 17•9 years ago
|
||
Alright, got a good try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2925a5fcce88
Assignee | ||
Comment 18•9 years ago
|
||
Comment on attachment 8733115 [details]
MozReview Request: Bug 1258154 - Open functions in variables view if they were returned from console evaluation;r=jlongster
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41561/diff/1-2/
Attachment #8733116 -
Attachment description: MozReview Request: Bug 1258154 - Remove extra global in browser_console_variables_view.js;r=jlongster → MozReview Request: Bug 1258154 - Minor test cleanups for function inspection in console;r=jlongster
Assignee | ||
Comment 19•9 years ago
|
||
Comment on attachment 8733116 [details]
MozReview Request: Bug 1258154 - Minor test cleanups for function inspection in console;r=jlongster
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41563/diff/1-2/
Assignee | ||
Comment 20•9 years ago
|
||
https://reviewboard.mozilla.org/r/41561/#review39971
::: devtools/client/webconsole/console-output.js:2782
(Diff revisions 1 - 2)
> this._text(")");
> },
>
> _onClick: function () {
> let location = this.objectActor.location;
> - if (location && location.source && location.source.url) {
> + if (location && IGNORED_SOURCE_URLS.indexOf(location.url) === -1) {
James, this is the change I made to get it to work: https://reviewboard.mozilla.org/r/41561/diff/1-2/. It's basically making sure that location.url is not "debugger eval code"
Assignee | ||
Comment 21•9 years ago
|
||
Does the change in Comment 20 seem reasonable?
Flags: needinfo?(jlong)
Comment 22•9 years ago
|
||
https://reviewboard.mozilla.org/r/41561/#review40073
I see, so the location on objects does not use the same location structure as sources. Hm, I'll think about if that's something we should fix in the future. For now this seems fine. (Sorry for the slow review)
Comment 23•9 years ago
|
||
Comment 24•9 years ago
|
||
Comment 25•9 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #21)
> Does the change in Comment 20 seem reasonable?
Yeah, sorry about that. I didn't realize ObjectActors use a different location object.
MozReview is *so* confusing... I've r+ed it like 3 times but there's still a pending issue? I have no idea. r+ from me on all fronts :)
Updated•9 years ago
|
Flags: needinfo?(jlong)
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 26•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/85199a7b852f
https://hg.mozilla.org/integration/fx-team/rev/712a70746d18
Keywords: checkin-needed
Comment 27•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/85199a7b852f
https://hg.mozilla.org/mozilla-central/rev/712a70746d18
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Comment 28•8 years ago
|
||
I have reproduced this bug with Nightly 48.0a1 (2016-03-19) on Windows 7 , 64 Bit!
But I could not see the fix on latest Beta 48.0b8 & latest Nightly and Developer Edition
Build ID 20160714050942
User Agent Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:48.0) Gecko/20100101 Firefox/48.0
Build ID 20160714030208
User Agent Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID 20160708004052
User Agent Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:49.0) Gecko/20100101 Firefox/49.0
Comment 29•8 years ago
|
||
(In reply to Maruf Rahman from comment #28)
> I have reproduced this bug with Nightly 48.0a1 (2016-03-19) on Windows 7 ,
> 64 Bit!
>
> But I could not see the fix on latest Beta 48.0b8 & latest Nightly and
> Developer Edition
>
>
>
>
> Build ID 20160714050942
>
> User Agent Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:48.0)
> Gecko/20100101 Firefox/48.0
>
>
> Build ID 20160714030208
>
> User Agent Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:50.0)
> Gecko/20100101 Firefox/50.0
>
>
> Build ID 20160708004052
>
> User Agent Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:49.0)
> Gecko/20100101 Firefox/49.0
Sorry, my mistake. This bug's fix is verified on latest Beta 48.0b8 & latest Nightly and Developer Edition
Reporter | ||
Updated•8 years ago
|
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•