Closed Bug 1375036 Opened 7 years ago Closed 5 years ago

The Javascript debugger does not follow the container tab session

Categories

(DevTools :: Debugger, defect, P3)

defect

Tracking

(firefox-esr60 wontfix, firefox-esr68 wontfix, firefox67- wontfix, firefox67.0.1- wontfix, firefox68- wontfix, firefox69- wontfix, firefox70 fixed)

RESOLVED FIXED
Firefox 70
Tracking Status
firefox-esr60 --- wontfix
firefox-esr68 --- wontfix
firefox67 - wontfix
firefox67.0.1 - wontfix
firefox68 - wontfix
firefox69 - wontfix
firefox70 --- fixed

People

(Reporter: mattia.basaglia, Assigned: bhackett1024)

References

(Blocks 2 open bugs)

Details

(Keywords: regression)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170612122310

Steps to reproduce:

* Log in or set a cookie that changes the page contents on a normal page
* Open a new container tab for the same page
* Open the Javascript debugger
* Select the page to view its sources


Actual results:

The sources of the current page show what you would get if you fetched the page from the non-container page. From the server log it seems it makes a new request and disregards the container tab.


Expected results:

It should instead show the page as fetched under the container environment.
Component: Untriaged → Developer Tools: Debugger
Thanks for the report! it seems like this might be a server issue, we will investigate further
If you want I made a simple page to help reproduce this: https://mattbas.org/misc/mozilla-bug-1375036/

It's a very simple PHP script and it prints its own sources so you can see what it's doing.

To repro the bug with this page:

* visit https://mattbas.org/misc/mozilla-bug-1375036/?foo=bar without a container
  (this sets a cookie that alters the contents of the page)
* open a new container
* visit https://mattbas.org/misc/mozilla-bug-1375036/ in that container

If you do that, on view-source it should show that there's a javascript variable called cookie set to '' (since the cookie is not set for the container.
But on the debugger the sources show cookie='bar' as it doesn't honor the container cookies.
Product: Firefox → DevTools
Yulia, what do you mean by "might be a server issue"? If you mean some type of server component within Firefox/Devtools, then I guess that's beyond my knowledge, but if you mean Mattia's HTTP server then you're incorrect. I have experienced this issue (in Firefox 62) and it has nothing to do with web servers. The debugger is literally just reloading the URL of the current (container) tab using the context of the default container and showing the source of a completely page.

This simply breaks the debugger tool completely when using containers, and has done for a year.
Hi Roy,

The comment was referring to the Debugger Server.
Right. So then, is this just a symptom of bug 1472581? (I didn't find an existing one that described what I saw, so reported it as new, but I would think it's known to someone.)
In a sense they are related. The issue is that we are not respecting headers when we are loading information in the debugger. I believe this is closely related to bug 1149835 and bug 1060732, specifically https://bugzilla.mozilla.org/show_bug.cgi?id=1060732#c3 -- in that we are not respecting the correct headers -- I will confirm with Jim and Alex regarding this.
Flags: needinfo?(poirot.alex)
Flags: needinfo?(jimb)
How does the debugger fetch the sources?

It has to go through DevToolsUtils.js's fetch helper in order to retrieve the right data:
  https://searchfox.org/mozilla-central/source/devtools/shared/DevToolsUtils.js#490-525
This helper method tries to ensure fetching the same sources the current tab is using.
The most important arguments of this method regarding the containers are `principal` and `window`.
You have to pass at least one of those and it to refer to the current debugged tab.

I see fetch() being used by the debugger actors over here:
  https://searchfox.org/mozilla-central/source/devtools/server/actors/source.js#410
Does the STR from comment 0 go throught this line?
If yes, is `principal` defined? Is principal.userContextId different from 0?
If yes, then, there is an issue somewhere inside fetch helper.
Flags: needinfo?(poirot.alex) → needinfo?(ystartsev)

I am able to reproduce this issue using the following STRs:

  1. Make sure to disable Cache in the Network panel toolbar.

  2. Load the following URL in it's own tab:
    https://mattbas.org/misc/mozilla-bug-1375036/?foo=cookie-value

-> you should see cookie-value in <h1>Cookie</h1> section at the top of the page

  1. Open the Debugger and check out the source of (index) file

-> the cookie-value is not in the source => BUG

Expected:
The source displayed in the Debugger should be the same as the debugged (current) page.

I believe that bug 1472581 is duplicate.

Honza

Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Blocks: dbg-sources
Version: 54 Branch → unspecified

Ever since version 67.0 this now affects every page in my application. Every page in my application redirects to my login page if the user does not have a valid session. Since the debugger now uses its own session ID, it shows the code that redirects to the login page.
<script type='text/javascript'>top.window.location.href='login.php?sx=1'</script>
Ultimately that means I am unable to debug anything except the login page.

Please expedite a fix for this as the debugger is an essential tool for all developers, myself included.

Brian, I'm curious if you can see how we night have regressed here. I've seen similar reports elsewhere.

Flags: needinfo?(bhackett1024)

I just confirmed that this worked in 66, but fails in nightly.

Has Regression Range: --- → yes
Has STR: --- → yes

Is this something important to fix before releasing trailhead? Or just a general issue we may want to aim to fix in 68 or maybe a 67 dot release?

Flags: needinfo?(jlaster)

Just a general issue we want to fix as soon as possible. Not sure if we need an uplift

Flags: needinfo?(jlaster)

Peter, it sounds like you're experiencing bug 1472581.

I'm not sure when this particular case regressed, but bug 1538056 should work as a partial fix for the more general problem, where the debugger refetches HTML sources and might get a different / changed result. After that bug lands, when the devtools are open when loading the page (including when reloading), the HTML source read off the network is used and no refetch is performed.

As for the more general problem of being able to open the debugger in an existing tab and see the right HTML source, I don't see how to fix fetch() to do the right thing 100% of the time. I see a couple other options:

  • Try to save HTML source contents, even when the devtools aren't open. This adds CPU and memory overhead for all users, not just those using the devtools, and tuning things like this can be pretty hard. Ultimately we'll have to discard the contents at some point (e.g. after a compacting GC, but probably sooner) so this method still won't do the right thing 100% of the time.

  • Compare the HTML source from fetch() with the source of the scripts themselves, which the JS engine has to retain (so that function.toString() works). If there is a mismatch, we could just display a note that the original HTML will be shown after reloading, instead of displaying a bogus HTML source. This option would be my preference.

Flags: needinfo?(bhackett1024)

(In reply to Brian Hackett (:bhackett) from comment #16)

As for the more general problem of being able to open the debugger in an existing tab and see the right HTML source, I don't see how to fix fetch() to do the right thing 100% of the time.

I believe that it would be enough to ensure that the request performed by the dev tools is exactly the same as the request performed to load the page. While this doesn't guarantee the sources will match 100% of the times (eg: if the page has time-sensitive content), it would at least work in a more intuitive way.

I want Firefox to be fast for everyone, and I enjoy developing in it. Having no HTML source scripts in the Debugger panel, or a warning that it may be inaccurate, unless Devtools is already open when the page is loaded, would be acceptable if it means performance is not affected for every user. Preferably, this saving of the exact source would be triggered by the Devtools being open in any tab/window. As I mentioned in bug 1472581, it should include any changes to the raw markup by document.write(), etc.

(In reply to mattia.basaglia from comment #17)

While this doesn't guarantee the sources will match 100% of the times (eg: if the page has time-sensitive content)

Or when it's a POST request. There are tons of issues in bugzilla related to the wrong sources being shown in the debugger. Bug 1161278 is another. Bug 1394478 is another that relates to just script files. These problems are going to keep coming up, and keep causing piles of web developer frustration until what we're trying to debug is what we are debugging. Everything else is a hack regardless of how well it's done.

(In reply to Roy Orbison from comment #18)

Having no HTML source scripts in the Debugger panel, or a warning that it may be inaccurate, unless Devtools is already open when the page is loaded, would be acceptable if it means performance is not affected for every user.

I agree with this approach. In computing, repeating the same action will often yield different results. I would be quite happy to live with the warning if deciding to debug after the page is already loaded, or to take the hit of reloading the page to get an accurate source. If devtools does have to do a fetch when it loads, first prize it does it in the same context (session cookie) to get the closest result.

In my case, my application tests every page request to ensure the user is logged in, and redirects if not. So without the session cookie being passed, it means I can't debug without disabling the test. I have disabled the test on my development server to get around it, so it's not a train smash for me. Still it would be nice to fix as I test almost exclusively in Firefox as my browser of choice.

Thanks, and yes, it seems bug 1472581 is related.

(In reply to Jason Laster [:jlast] from comment #12)

I just confirmed that this worked in 66, but fails in nightly.

This bug was filed two years ago though?

Julien, My guess is that we fixed it before we broke it recently.

Depends on: 1538056
Flags: needinfo?(bhackett1024)
Flags: needinfo?(bhackett1024)

I am using Firefox 69.0b11 64-bit and the bug is still present. I too regularly need to debug pages that are behind a login and the only option I have is to use Chrome - please fix this as soon as possible. Thanks.

(In reply to maxscan from comment #24)

I am using Firefox 69.0b11 64-bit and the bug is still present. I too regularly need to debug pages that are behind a login and the only option I have is to use Chrome - please fix this as soon as possible. Thanks.

Hi, can you try using a nightly build? Bug 1538056 landed a few days ago and is only in nightly builds. That bug should fix this issue, though only when the page is visited / reloaded while the devtools are open (per comment 16).

Still seeing the same behaviour in Nightly as in https://bugzilla.mozilla.org/show_bug.cgi?id=1556969.
With or without reloading when the dev-tools are open, scripts still cannot be debugged.

The nightly loads the session source, this bug in its literal context is essentially fixed.
But:

  • Errors in console lead to plain text representation of source where one can't add break points
  • Before document ready state, request response exceptions are not paused upon. Even if you add a break point on the same line the error occurs by manually navigating to the file and line no in the debugger (due to no. 2), it will not function before ready state is achieved.

Sorry the second point should reference no. 1.

Clean ni?s

Lets file follow up bugs for the 2 bullets from comment 27

Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(ystartsev)
Flags: needinfo?(jimb)
Resolution: --- → FIXED

(In reply to Umair Ahmed from comment #27)

The nightly loads the session source, this bug in its literal context is essentially fixed.
But:

  • Errors in console lead to plain text representation of source where one can't add break points
  • Before document ready state, request response exceptions are not paused upon. Even if you add a break point on the same line the error occurs by manually navigating to the file and line no in the debugger (due to no. 2), it will not function before ready state is achieved.

I don't understand this comment.
Umair, can you please provide some STRs explaining the two points above?
I am happy to verify and file follow ups.
Honza

Flags: needinfo?(syedumairahmed)
Assignee: nobody → bhackett1024
Target Milestone: --- → Firefox 70

Well I am currently too busy too provide some test case for the 2nd case as it needs a bug introduced before ready state is achieved. The first one is something I encounter regularly but still requires a bug or error in console with clear reference to originating JS file.

For example, if you have a bug or error thrown in a JS file, and it is shown in console:

When clicking on any of the references (any file in the stack or the origin) will lead to a plain text view of that file with the reference highlighted:

Flags: needinfo?(syedumairahmed)

Sorry for some reason, the markdown didn't work to include images you can find them here:
https://imgur.com/5HQaEIW
https://imgur.com/jAQ35Pe

Another example can be taken from our very own MDN website where some errors show up due to MIME type mismatch (https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/time) and half of their origin references, though tooltipped with "View source in debugger", instead open in a plain text view in a new tab...

Honza, with the comments above – should we re-open a follow-up bug?

(please see comment 35)

Flags: needinfo?(odvarko)
Flags: needinfo?(odvarko)
You need to log in before you can comment on or make changes to this bug.