Bug 61868 - Web Inspector: ResourceCookiesView.resize() missing
Summary: Web Inspector: ResourceCookiesView.resize() missing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Andrey Kosyakov
URL:
Keywords: Regression
Depends on:
Blocks:
 
Reported: 2011-06-01 09:49 PDT by Andrey Kosyakov
Modified: 2011-06-02 18:21 PDT (History)
11 users (show)

See Also:


Attachments
patch (1.17 KB, patch)
2011-06-01 09:52 PDT, Andrey Kosyakov
pfeldman: review-
Details | Formatted Diff | Diff
patch (1.21 KB, patch)
2011-06-02 04:09 PDT, Andrey Kosyakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Kosyakov 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'
Comment 1 Andrey Kosyakov 2011-06-01 09:52:10 PDT
Created attachment 95615 [details]
patch
Comment 2 Pavel Feldman 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?
Comment 3 Andrey Kosyakov 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.
Comment 4 Pavel Feldman 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?
Comment 5 Andrey Kosyakov 2011-06-02 04:09:26 PDT
Created attachment 95750 [details]
patch
Comment 6 Pavel Feldman 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...
Comment 7 WebKit Commit Bot 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>
Comment 8 WebKit Commit Bot 2011-06-02 18:21:43 PDT
All reviewed patches have been landed.  Closing bug.