WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
26145
WebInspector halts when opening with debugger enabled globally
https://bugs.webkit.org/show_bug.cgi?id=26145
Summary
WebInspector halts when opening with debugger enabled globally
Pavel Feldman
Reported
2009-06-02 11:51:28 PDT
1. Open Web Inspector 2. Go to Scripts tab and choose 'Always enabled' 3. Restart browser 4. Open Web Inspector Expected: window opens Actual: window opens and gets into non-responsive mode. Browser halts.
Attachments
fix
(8.85 KB, patch)
2009-06-03 09:27 PDT
,
Pavel Feldman
timothy
: review+
Details
Formatted Diff
Diff
fix
(8.36 KB, patch)
2009-06-03 10:08 PDT
,
Pavel Feldman
timothy
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Pavel Feldman
Comment 1
2009-06-03 09:27:10 PDT
Created
attachment 30909
[details]
fix
Timothy Hatcher
Comment 2
2009-06-03 09:37:48 PDT
Comment on
attachment 30909
[details]
fix
> - enableDebugger(); > + m_frontend->attachDebuggerWhenShown();
Can't you just leave this calling enableDebugger() since you fixed that function?
> disableDebugger(); > + m_attachDebuggerWhenShown = true;
Why is this needed? Shoudln't this be m_attachDebuggerWhenShown = false? If it is supose to be false, should disableDebugger() set it to false? If it isn't those things, maybe a comment is needed to explain.
> - enableDebugger(); > + m_attachDebuggerWhenShown = true;
Same as before. Can't you just leave this calling enableDebugger() since you fixed that function?
> + this._attachDebuggerWhenShown = false;
I like to use delete this._attachDebuggerWhenShown for cases like this, since there is no benifit to keeping this property around. r+, but maybe this can be a smaller change based on the comments above.
Pavel Feldman
Comment 3
2009-06-03 10:08:12 PDT
Created
attachment 30911
[details]
fix (In reply to
comment #2
)
> (From update of
attachment 30909
[details]
[review]) > > - enableDebugger(); > > + m_frontend->attachDebuggerWhenShown(); > > Can't you just leave this calling enableDebugger() since you fixed that > function? >
Done
> > disableDebugger(); > > + m_attachDebuggerWhenShown = true; > > Why is this needed? Shoudln't this be m_attachDebuggerWhenShown = false? If it > is supose to be false, should disableDebugger() set it to false? If it isn't > those things, maybe a comment is needed to explain. > > > - enableDebugger(); > > + m_attachDebuggerWhenShown = true; >
I added a comment for this. The scenario is: close window when debugger is enabled, expect it to be enabled when the window is being reopened. The hidden thing is that debugger is being disabled on window close unconditionally.
> Same as before. Can't you just leave this calling enableDebugger() since you > fixed that function? >
Done. It is just that I fixed the function after I changed these.
> > + this._attachDebuggerWhenShown = false; > > I like to use delete this._attachDebuggerWhenShown for cases like this, since > there is no benifit to keeping this property around. >
Done.
> r+, but maybe this can be a smaller change based on the comments above. >
Thanks!
Brent Fulgham
Comment 4
2009-06-03 15:26:18 PDT
Assign for landing.
Brent Fulgham
Comment 5
2009-06-03 15:37:32 PDT
Landed in @
r44398
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug