RESOLVED FIXED61868
Web Inspector: ResourceCookiesView.resize() missing
https://bugs.webkit.org/show_bug.cgi?id=61868
Summary Web Inspector: ResourceCookiesView.resize() missing
Andrey Kosyakov
Reported 2011-06-01 09:49:46 PDT
1. Open inspector 2. Navigate to http://www.webkit.org 3. Switch to network panel 4. Select main resource 5. Switch to cookies view 6. Resize WebInspector window Observer that columns can no longer be resized, the following error appears on inspector's console: Uncaught TypeError: Object #<Object> has no method 'resize'
Attachments
patch (1.17 KB, patch)
2011-06-01 09:52 PDT, Andrey Kosyakov
pfeldman: review-
patch (1.21 KB, patch)
2011-06-02 04:09 PDT, Andrey Kosyakov
no flags
Andrey Kosyakov
Comment 1 2011-06-01 09:52:10 PDT
Pavel Feldman
Comment 2 2011-06-01 12:34:03 PDT
Comment on attachment 95615 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=95615&action=review > Source/WebCore/inspector/front-end/ResourceCookiesView.js:64 > + this._cookiesTable.updateWidths(); Can resize() be called before show()? I.e. are you missing the null check here?
Andrey Kosyakov
Comment 3 2011-06-01 14:49:54 PDT
(In reply to comment #2) > (From update of attachment 95615 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=95615&action=review > > > Source/WebCore/inspector/front-end/ResourceCookiesView.js:64 > > + this._cookiesTable.updateWidths(); > > Can resize() be called before show()? I.e. are you missing the null check here? So far we only call it in case the view is already visible (http://www.google.com/codesearch?hl=en&vert=chromium&lr=&q=cookiesView.resize&sbtn=Search). Whether we should do a guard check for future usages is a matter of taste -- I'd rather not. We shouldn't be resizing something that we're not showing at the moment.
Pavel Feldman
Comment 4 2011-06-01 22:22:39 PDT
Comment on attachment 95615 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=95615&action=review >>> Source/WebCore/inspector/front-end/ResourceCookiesView.js:64 >>> + this._cookiesTable.updateWidths(); >> >> Can resize() be called before show()? I.e. are you missing the null check here? > > So far we only call it in case the view is already visible (http://www.google.com/codesearch?hl=en&vert=chromium&lr=&q=cookiesView.resize&sbtn=Search). Whether we should do a guard check for future usages is a matter of taste -- I'd rather not. We shouldn't be resizing something that we're not showing at the moment. ResourceCookiesView inherits from View, so it should probably behave as a view. Views can get resize signals at any time while visible. I don't see synchronous correlation between showing view and getting this._cookiesTable assigned. In fact, if our tabbed pane was properly implemented and did delegate resize to the tabs, your code would fail on the cookie-less resource resize, right?
Andrey Kosyakov
Comment 5 2011-06-02 04:09:26 PDT
Pavel Feldman
Comment 6 2011-06-02 11:10:42 PDT
Comment on attachment 95750 [details] patch We probably still want to fix tabbed pane so that it was a view and was calling resize of the selected view...
WebKit Commit Bot
Comment 7 2011-06-02 18:21:38 PDT
Comment on attachment 95750 [details] patch Clearing flags on attachment: 95750 Committed r87981: <http://trac.webkit.org/changeset/87981>
WebKit Commit Bot
Comment 8 2011-06-02 18:21:43 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.