Summary: | REGRESSION (r46972): Inspector console outputs the global value for a variable instead of the local value | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, ggaren, joepeck, mjs, oliver, pfeldman, socket.h, spamfaenger | ||||||||
Priority: | P2 | Keywords: | HasReduction, InRadar, Regression | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Bug Depends on: | 28078 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
The bisect-builds script reports: Works: r47686 Fails: r48034 Breakpoints don't seem to work in the Inspector between r47686-r48004, so I couldn't narrow it down in that range. CC-ing some Web Inspector developers. *** Bug 32513 has been marked as a duplicate of this bug. *** This appears to be a JavaScriptCore bug, not a Web Inspector bug, since it doesn't reproduce on Google Chrome for Mac 4.0.249.49. Still occurs with WebKit nightly build r53845. No longer reproduces in the current nightly (In reply to comment #5) > No longer reproduces in the current nightly I still see "This is the global value" (which is the broken behavior) with WebKit nightly r54122. Which nightly are you using, Oliver? (In reply to comment #6) > (In reply to comment #5) > > No longer reproduces in the current nightly > > I still see "This is the global value" (which is the broken behavior) with > WebKit nightly r54122. > > Which nightly are you using, Oliver? Wrong bug sorry :-( Any progress here? I just spent another day hunting a bug in the wrong direction because of this. :/ Is there any help that you need? (Another testcase reduction perhpas?) Created attachment 51845 [details]
Proposed fix v1.0
Comment on attachment 51845 [details] Proposed fix v1.0 Looks great! One nit and we can land this! > + if (!dontUseCommandLineAPI) { > + expression = "with (window.console._inspectorCommandLineAPI) { with (window) {\n" + expression + "\n} }"; > + } Single-line conditions should not have {}. Created attachment 51846 [details]
Proposed fix v1.1
Comment on attachment 51846 [details] Proposed fix v1.1 Clearing flags on attachment: 51846 Committed r56676: <http://trac.webkit.org/changeset/56676> All reviewed patches have been landed. Closing bug. Comment on attachment 51846 [details] Proposed fix v1.1 >+ Fix the regression caused by r28078: a global variable >+ definition masks a local one in an inspector console >+ https://bugs.webkit.org/show_bug.cgi?id=32442 r28078 doesn't seem to the be revision that caused the regression. <http://trac.webkit.org/changeset/28078> (In reply to comment #15) > (From update of attachment 51846 [details]) > >+ Fix the regression caused by r28078: a global variable > >+ definition masks a local one in an inspector console > >+ https://bugs.webkit.org/show_bug.cgi?id=32442 > > r28078 doesn't seem to the be revision that caused the regression. > <http://trac.webkit.org/changeset/28078> Ah, I think you mean Bug 28078 which was fixed by r46972. <http://trac.webkit.org/changeset/46972> Nice job on the fix, Dmitry! Is it possible to write a LayoutTests/inspector test for this so it doesn't happen again? (In reply to comment #17) > Nice job on the fix, Dmitry! > > Is it possible to write a LayoutTests/inspector test for this so it doesn't > happen again? Technically, it is possible. However, we don't yet have layout tests that stop in debugger, so various problems are likely to arise. Scenario for the test would be: - In the layout test page, create a function that has debugger; statement. That's pretty much all you do on the layout page side, rest is happening on the inspector front-end. - On the front-end side, enable debugger (this is done synchronously) and execute the function you created in previous step using InjectedScriptAccess.evaluate. - Just before you do this evaluate, add a sniffer that would give your code control upon WebInspector.pausedScript call from the backend. - As soon as pausedScript is called, get a reference to the call frame. - Either synchronously or in setTimeout(0) execute InjectedScriptAccess.evaluateInCallFrame against your call frame with "x" expression. tests are located under LayoutTests/inspector |
Created attachment 44701 [details] Test case * SUMMARY When breaking inside a function with a local variable that masks a global variable, printing the variable shows the globally-scoped variable instead of the locally-scoped one. * STEPS TO REPRODUCE 1. Launch Safari or WebKit nightly. 2. Enable JavaScript Debugging in the Web Inspector. 3. Load the test page. 4. Type 'x' in the console and hit Enter. * EXPECTED RESULTS The console should print: "This is the local value" * ACTUAL RESULTS This console prints: "This is the global value" * REGRESSION This is a regression from shipping Safari 4.0.4.